From f032821e5d6ca8787d8c9d9a683ab3f688beed53 Mon Sep 17 00:00:00 2001 From: schneems Date: Wed, 15 Jun 2016 17:05:06 -0500 Subject: [PATCH] Prefer Hash#[] over Set#.include? for speed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Playing with stackprof against codetriage and for an initial run with no cache `Set#include?` was the top called method, something around 8% of execution time spent there. Did a microbenchmark to see if it would be faster to use a hash: ``` require 'benchmark/ips' require 'set' set = Set.new [:foo, :bar] hash = {foo: true, bar: true} Benchmark.ips do |x| x.report("set ") {|num| i = 0 while i < num set.include?(:foo) i += 1 end } x.report("hash") {|num| i = 0 while i < num hash[:foo] i += 1 end } x.compare! end # Warming up -------------------------------------- # set 215.314k i/100ms # hash 219.939k i/100ms # Calculating ------------------------------------- # set 11.715M (±15.5%) i/s - 56.843M in 5.010837s # hash 20.119M (±18.2%) i/s - 96.333M in 5.010977s # Comparison: # hash: 20118880.7 i/s # set : 11714839.0 i/s - 1.72x slower ``` Yes, it is faster. Anecdotally when running `RAILS_ENV=production time rake assets:precompile` against codetriage: Before patch: ``` eal 0m18.325s user 0m14.564s sys 0m2.729s ``` After patch: ``` real 0m17.981s user 0m14.461s sys 0m2.716s ``` --- lib/sprockets/processor_utils.rb | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/lib/sprockets/processor_utils.rb b/lib/sprockets/processor_utils.rb index f8c1e53a3..d47bd61b5 100644 --- a/lib/sprockets/processor_utils.rb +++ b/lib/sprockets/processor_utils.rb @@ -130,6 +130,17 @@ def processors_cache_keys(processors) Set ]).freeze + # Internal: Hash of all "simple" value types allowed to be returned in + # processor metadata. + VALID_METADATA_VALUE_TYPES_HASH = VALID_METADATA_VALUE_TYPES.each_with_object({}) do |type, hash| + hash[type] = true + end.freeze + + # Internal: Hash of all nested compound metadata types that can nest values. + VALID_METADATA_COMPOUND_TYPES_HASH = VALID_METADATA_COMPOUND_TYPES.each_with_object({}) do |type, hash| + hash[type] = true + end.freeze + # Internal: Set of all allowed metadata types. VALID_METADATA_TYPES = (VALID_METADATA_VALUE_TYPES + VALID_METADATA_COMPOUND_TYPES).freeze @@ -168,9 +179,9 @@ def validate_processor_result!(result) # # Returns true if class is in whitelist otherwise false. def valid_processor_metadata_value?(value) - if VALID_METADATA_VALUE_TYPES.include?(value.class) + if VALID_METADATA_VALUE_TYPES_HASH[value.class] true - elsif VALID_METADATA_COMPOUND_TYPES.include?(value.class) + elsif VALID_METADATA_COMPOUND_TYPES_HASH[value.class] value.all? { |v| valid_processor_metadata_value?(v) } else false