-
Notifications
You must be signed in to change notification settings - Fork 312
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
Hashie::Mash deep_merge is super O(2^n) slow #106
Comments
The call to custom_writer within deep_merge was exponentially converting the values (unnecessarily, since they were just converted within deep_merge). Added an optional param to custom_writer to suppress the conversion. Unfortunately, had to also add the param to coercion's customer_writer method as well (which is just ignored) to not break the inheritance. All tests pass.
This issue is actually critical. Without this patch, a request with just 10 elements of array takes more than 30 seconds. I strongly recommend to merge this patch and release new version of Hashie asap. We are using the @davemitchell's forked repo until this is merged. (thank you so much for this patch, @davemitchell !) |
Yeah I second that. Just did a bundle update on another gem and along came Hashie 2.0.5 - specs became 10x slower all because of Hashie. Took me two hours to understand what happened. |
@jch @wapcaplet @markiz @craiglittle @7even @jimeh @davemitchell hey guys this issue is a big one really, this pull request shouldn't just be sitting here for months. 2.0.5 is unusable in it's current state. |
Sorry, I was helping, but I lost access after leaving Intridea. The project On Tuesday, October 8, 2013, John Axel Eriksson wrote:
-Jerry |
I am taking over. The fix has now been merged on master and I will make a release this week after I've gone through all the pending PRs. |
sweet! |
Mash's
deep_merge
is recursively invoked 2^n times, where n is the depth of theother_hash
parameter. For instance,...will call
deep_merge
over a million times (2^20), taking 10-15 seconds to complete.I've made an a O(n) fix in which
deep_merge
is only invoked 20 times for the above example, with sub-millisecond response. Will submit a pull request momentarily.The text was updated successfully, but these errors were encountered: