From c3f00e3e8d67ff5d6470315ad137166177f079e2 Mon Sep 17 00:00:00 2001 From: Marc Siegel Date: Wed, 22 Mar 2017 10:24:56 -0400 Subject: [PATCH] Optimize `#with` This PR is inspired by https://github.com/tcrayford/Values/pull/56, and assumes that code will be merged, so uses it in the benchmarks here: https://gist.github.com/ms-ati/fa8002ef8a0ce00716e9aa6510d3d4d9 It is common in our code, as in any idiomatic code using value objects in loops or pipelines, to call `#with` many times, returning a new immutable object each time with 1 or more fields replaced with new values. The optimizations in this PR eliminate a number of extra Hash and Array instantiations that were occurring each time, in favor of iterating only over the constant `VALUE_ATTRS` array and doing key lookups in the given Hash parameter in the hot paths. Per the gist above, this increases ips (iterations per second) 2.29x, from 335.9 to 769.6 on my machine. --- lib/values.rb | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/lib/values.rb b/lib/values.rb index 3c8af8a..6bc5874 100644 --- a/lib/values.rb +++ b/lib/values.rb @@ -49,13 +49,15 @@ def initialize(*values) const_set :VALUE_ATTRS, fields def self.with(hash) - unexpected_keys = hash.keys - self::VALUE_ATTRS - if unexpected_keys.any? + num_recognized_keys = self::VALUE_ATTRS.count { |field| hash.key?(field) } + + if num_recognized_keys != hash.size + unexpected_keys = hash.keys - self::VALUE_ATTRS raise ArgumentError.new("Unexpected hash keys: #{unexpected_keys}") end - missing_keys = self::VALUE_ATTRS - hash.keys - if missing_keys.any? + if num_recognized_keys != self::VALUE_ATTRS.size + missing_keys = self::VALUE_ATTRS - hash.keys raise ArgumentError.new("Missing hash keys: #{missing_keys} (got keys #{hash.keys})") end @@ -94,9 +96,22 @@ def pretty_print(q) end end + # Optimized to avoid intermediate Hash instantiations. def with(hash = {}) return self if hash.empty? - self.class.with(to_h.merge(hash)) + + num_recognized_keys = self.class::VALUE_ATTRS.count { |field| hash.key?(field) } + + if num_recognized_keys != hash.size + unexpected_keys = hash.keys - self.class::VALUE_ATTRS + raise ArgumentError.new("Unexpected hash keys: #{unexpected_keys}") + end + + args = self.class::VALUE_ATTRS.map do |field| + hash.key?(field) ? hash[field] : send(field) + end + + self.class.new(*args) end def to_h