Skip to content

Commit

Permalink
Prefer Hash#[] over Set#.include? for speed
Browse files Browse the repository at this point in the history
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
```
  • Loading branch information
schneems committed Jun 15, 2016
1 parent 26c090a commit f032821
Showing 1 changed file with 13 additions and 2 deletions.
15 changes: 13 additions & 2 deletions lib/sprockets/processor_utils.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down

0 comments on commit f032821

Please sign in to comment.