-
Notifications
You must be signed in to change notification settings - Fork 807
Addressing #838, STI issues when code is reloaded in development, fix... #1064
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
...nt, fix index name to klass lookup methodology
💚 CLA has been signed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I put Benchmark.measure around this code and contrary to my expectations, the original hash method was WAY. We will have to re-think this change, maybe the method of storing the class->index_name mapping can be improved to mitigate #838 with performance.
image
NOTE: my models use a Proc for
index_namein a multi-tenant application so we are attempting to use an apartment qualified index name. Seems the Proc based index_name is causing the performance hit.
...ic#838 and improve autoloader compatibility
The class level map using @@__types messes up the autoloading when STI classes are present. The Registry.all.detect lookup should be very fast so this seems to be a good trade off to fix the old STI auto load issue that was reported in #838 and closed by the stalebot.
Related PR #888 was also closed with a statement that STI support is deprecated and removed from the gem. It is my opinion that this PR and #888 is a fix related to Rails autoloading and has nothing to do with STI support.
Please consider this PR to close #848 for good. Thanks!