Use compiled expression in AliasToBeanResultTransformer #2021
Conversation
|
We can cache the compiled delegate by aliases to avoid breaking changes. |
|
Yeah. But is it worth it? Current transformer design means that lookup is required on each record transformation. |
src/NHibernate.Test/TransformTests/AliasToBeanResultTransformerFixture.cs
Outdated
Show resolved
Hide resolved
| @@ -310,6 +334,8 @@ public bool Equals(AliasToBeanResultTransformer other) | |||
| { | |||
| return true; | |||
| } | |||
| if (GetType() != other.GetType()) | |||
| return false; | |||
| return Equals(other._resultClass, _resultClass); | |||
There was a problem hiding this comment.
Equality is somewhat troublesome with this PR changes. Can we really consider equal two AliasToBeanResultTransformer having been used for different queries? They will not be usable on the same queries, and so are not equivalent.
I think equality here needs to be changed for reference equality only. Or @hazzik suggestion, caching _transformer to be used by aliases, needs to be done.
There was a problem hiding this comment.
I think equality here needs to be changed for reference equality only.
This would not be enough.
There was a problem hiding this comment.
Well I feel like it's either better to provide a separate class for compiled version or just close this PR and leave it be reflection based. As I don't like this alias caching idea - it unnecessary complicates things (eg. should this caching be thread safe? Looks like it should to support possible static instance of transformer)
Possible breaking change:
As now expression is compiled on first transformation - this transformer can't be reused for different queries. Transfomer can still be reused but only for the same query.
Perf comparison for 5 aliases and 50 000 transformations on my machine (see #2015 with side by side comparison):
300 ms vs 15 ms for Compiled version