-
Notifications
You must be signed in to change notification settings - Fork 936
NH-3489 improve GetEffectiveParameterLocations performance #214
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
Conversation
Thanks for the contribution, @MrJul. But I would like to have more explicit solution |
@hazzik what to do mean by "more explicit"? After all, I'm explicitly caching :) I'm really interested in getting this solved, this affects so many queries in one of our application that I've been maintaining a custom build for more than a year. |
I mean that I do not like the fact that you hide your magic cache behind |
It was to keep maximum backward compatibility for callers using it as a list. There's no observable side effect for them, in fact it could even become an option if necessary (use a plain List or the Cache list behind the IList). |
Ok, @MrJul. Can you please rebase on top of master branch? |
Sorry for the delay, that's done! |
@MrJul SharpTestEx has been removed on master, can you please update tests? |
@hazzik I don't get it, I don't use SharpTestEx in my tests for NH-3489. The compilation seems to already fail in the current NH master at https://github.com/nhibernate/nhibernate-core/blob/master/src/NHibernate.Test/NHSpecificTest/NH3570/UniFixture.cs |
Ok, sorry then |
@MrJul sorry for asking, but could you please rebase on top of master again? |
No problem @hazzik that's done! |
Added a cache of backtrace-to-location with the
BackTrackCacheParameterList
class, that is used bySqlCommandImpl
.GetEffectiveParameterLocations
still use the old code if a standard collection is passed for compatibility reasons.The only public change is
SqlCommandImpl.SqlQueryParametersList
that now has a return type ofIList
rather thanList
.A performance test case has been added. A ~40% performance improvement has been measured for the test scenario.