Fixes Issue #106: deep_merge recursively slow #107
Conversation
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.
|
I was just made aware of this issue via @johnae's comment on #106. Thankfully I haven't had any massive performance issues myself, but I'll probably switch for your fork for good measure till this is merged in. @davemitchell, I'd suggest maybe adding a test or two to ensure the behavior doesn't return further down the road. Specially since it's not exactly the most obvious of issues like a syntax error or something else silly. |
|
This patch worked like a charm. A |
|
Opened #116 with this change and a spec. |
|
Came across this issue by profiling with ruby-prof - why has it not been merged yet? 😟 |
|
cc: @mbleigh |
|
Hi @morganchristiansson, |
|
I now have access to the repo, am taking care of cleaning up the backlog and making a release. This has now been merged. |
|
@bartoszkopinski What other PRs did you merge into your fork? I want to look at them all. |
|
@dblock lol, my timing... nice job. :-) |
|
Well, your note prompted me to find @mbleigh :) Let me take some PRs, I'll diff your changes last. |
${RUBY_PKGPREFIX}-hashie2.
## 2.1.2 (5/12/2014)
* [#169](hashie/hashie#169): Hash#to_hash will also convert nested objects that implement `to_hash` - [@gregory](https://github.com/gregory).
## 2.1.1 (4/12/2014)
* [#144](hashie/hashie#144): Fixed regression invoking `to_hash` with no parameters - [@mbleigh](https://github.com/mbleigh).
## 2.1.0 (4/6/2014)
* [#134](hashie/hashie#134): Add deep_fetch extension for nested access - [@tylerdooling](https://github.com/tylerdooling).
* Removed support for Ruby 1.8.7 - [@dblock](https://github.com/dblock).
* Ruby style now enforced with Rubocop - [@dblock](https://github.com/dblock).
* [#138](hashie/hashie#138): Added Hashie::Rash, a hash whose keys can be regular expressions or ranges - [@epitron](https://github.com/epitron).
* [#131](hashie/hashie#131): Added IgnoreUndeclared, an extension to silently ignore undeclared properties at intialization - [@righi](https://github.com/righi).
* [#136](hashie/hashie#136): Removed Hashie::Extensions::Structure - [@markiz](https://github.com/markiz).
* [#107](hashie/hashie#107): Fixed excessive value conversions, poor performance of deep merge in Hashie::Mash - [@davemitchell](https://github.com/dblock), [@dblock](https://github.com/dblock).
* [#69](hashie/hashie#69): Fixed assigning multiple properties in Hashie::Trash - [@einzige](https://github.com/einzige).
* [#100](hashie/hashie#100): IndifferentAccess#store will respect indifference - [@jrochkind](https://github.com/jrochkind).
* [#103](hashie/hashie#103): Fixed support for Hashie::Dash properties that end in bang - [@thedavemarshall](https://github.com/thedavemarshall).
* [89](hashie/hashie#89): Do not respond to every method with suffix in Hashie::Mash, fixes Rails strong_parameters - [@Maxim-Filimonov](https://github.com/Maxim-Filimonov).
* [#110](hashie/hashie#110): Correctly use Hash#default from Mash#method_missing - [@ryansouza](https://github.com/ryansouza).
* [#120](hashie/hashie#120): Pass options to recursive to_hash calls - [@pwillett](https://github.com/pwillett).
* [#113](hashie/hashie#113): Fixed Hash#merge with Hashie::Dash - [@spencer1248](https://github.com/spencer1248).
* [#99](hashie/hashie#99): Hash#deep_merge raises errors when it encounters integers - [@defsprite](https://github.com/defsprite).
* [#133](hashie/hashie#133): Fixed Hash##to_hash with symbolize_keys - [@mhuggins](https://github.com/mhuggins).
* [#130](hashie/hashie#130): IndifferentAccess now works without MergeInitializer - [@npj](https://github.com/npj).
* [#111](hashie/hashie#111): Trash#translations correctly maps original to translated names - [@artm](https://github.com/artm).
* [#129](hashie/hashie#129): Added Trash#permitted_input_keys and inverse_translations - [@artm](https://github.com/artm).
|
I should probably update the hashie version. In other news, I'm looker for new maintainers if anyone in this thread is interested. |
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.