Skip to content

Replace an O(n) lookup in LINQ query parsing by an O(1) one #1869

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

fredericDelaporte
Copy link
Member

@fredericDelaporte fredericDelaporte commented Oct 6, 2018

The HQL generators registry already uses dictionaries to resolve generators for methods found in LINQ expression. But some generators do not declare statically their supported methods and instead have
to be queried with a loop on them for support of methods encountered in the LINQ query.

The result of this lookup can be cached for avoiding calling this loop again on the next query using the method.
If #1868 is merged, this change will compensate for #1868 causing up to three such lookups by method.

hazzik
hazzik previously approved these changes Oct 7, 2018
The HQL generators registry already uses dictionaries to resolve
generators for methods found in LINQ expression. But some generators
do not declare statically their supported methods and instead have
to be queried with a loop on them for support of methods encountered
in the LINQ query.

The result of this lookup can be cached for avoiding calling this loop
again on the next query using the method.
If nhibernate#1868 is merged, this change will compensate for nhibernate#1868 causing up to
three such lookups by method.
@fredericDelaporte
Copy link
Member Author

I have included the missing async trivial regen from another PR.

@fredericDelaporte fredericDelaporte added this to the 5.2 milestone Oct 7, 2018
@fredericDelaporte fredericDelaporte merged commit a58a56c into nhibernate:master Oct 7, 2018
@fredericDelaporte fredericDelaporte deleted the CacheRuntimeGenerator branch October 7, 2018 11:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants