Skip to content
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

Speed up digests with refinements #417

Closed
wants to merge 2 commits into from

Conversation

schneems
Copy link
Member

@schneems schneems commented Nov 4, 2016

Huge thank you to @bouk who did all the work here. This is a follow up from the conversation at #383 (comment).

Running asset generation in a loop on codetriage.com

task "assets:bench" do
  measure = []

  50.times do |i|
    puts "== Running (#{i})"
    measure << Benchmark.measure do
      `rm -rf tmp/cache/assets/sprockets/v4.0/ ; rm -rf public/assets; touch tmp; time RAILS_ENV=production bundle exec rake assets:precompile`
    end.real
  end
  puts "================ DONE ================"
  puts measure.join("\n")
end

I get these numbers:

            Avg         STDev
Refinements 10.06318182 0.2621523045
Hash        12.08513782 1.944694921

Based on this refinements is faster than the current hash (8-16% in a real world assets:precompile app), but there is a high variance.

Huge thank you to @bouk who did all the work here.

Running asset generation in a loop on codetriage.com

```
task "assets:bench" do
  measure = []

  50.times do |i|
    puts "== Running (#{i})"
    measure << Benchmark.measure do
      `rm -rf tmp/cache/assets/sprockets/v4.0/ ; rm -rf public/assets; touch tmp; time RAILS_ENV=production bundle exec rake assets:precompile`
    end.real
  end
  puts "================ DONE ================"
  puts measure.join("\n")
end
```

I get these numbers:

```
            Avg         STDev
Refinements 10.06318182 0.2621523045
Hash        12.08513782 1.944694921
```

Based on this refinements is faster than the current hash (8-16% in a real world assets:precompile app), but there is a high variance.
@schneems
Copy link
Member Author

schneems commented Nov 4, 2016

Ruby 2.0 uses a different key word http://ruby-doc.org/core-2.0.0/doc/syntax/refinements_rdoc.html

@bouk
Copy link
Member

bouk commented Nov 4, 2016

The minimum ruby version for rails 5 is 2.2.2, we ought to bump the minimum version of Sprockets to the same

@schneems
Copy link
Member Author

schneems commented Nov 4, 2016

I'm fine with that cc/ @rafaelfranca

@headius
Copy link

headius commented Nov 8, 2016

I need to jump in here and state that I think Refinements are the last place we want to look for improvement here. It may be faster than procs, but it will be slower than regular methods and it imposes a severe optimization boundary on implementations with optimizing compilers.

@headius
Copy link

headius commented Nov 8, 2016

Ok, so here's two examples that show how easy this is to optimize on JRuby, and I'm not even eliminating the hash. The first uses a plain lambda, and the second uses a #method-ized define_method method.

hash = {}
hash[:foo] = ->(a, b) { a || b }
loop do
  t = Time.now
  i = 0
  while i < 10_000_000
    i+=1
    hash.fetch(:foo).call 1, 2
    hash.fetch(:foo).call 1, 2
    hash.fetch(:foo).call 1, 2
  end
  puts Time.now - t
end
hash = {}
hash[:foo] = method define_method(:anon) {|a, b| a || b}
loop do
  t = Time.now
  i = 0
  while i < 10_000_000
    i+=1
    hash.fetch(:foo).call 1, 2
    hash.fetch(:foo).call 1, 2
    hash.fetch(:foo).call 1, 2
  end
  puts Time.now - t
end

And here's numbers on JRuby 9.1.6.0 versus MRI 2.3.1 on the lambda version (the define_method version is about 25% slower on MRI, of course):

[] ~/projects $ jruby -e "hash = {}; hash[:foo] = ->(a, b) { a || b }; loop { t = Time.now; i = 0; while i < 10_000_000; i+=1; hash.fetch(:foo).call 1, 2; hash.fetch(:foo).call 1, 2; hash.fetch(:foo).call 1, 2; end; puts Time.now - t }"
2.593
2.354
2.397
2.347
^C
[] ~/projects $ jruby -e "hash = {}; hash[:foo] = method define_method(:anon) {|a, b| a || b}; loop { t = Time.now; i = 0; while i < 10_000_000; i+=1; hash.fetch(:foo).call 1, 2; hash.fetch(:foo).call 1, 2; hash.fetch(:foo).call 1, 2; end; puts Time.now - t }"
1.745
1.568
1.509
1.568
^C
[] ~/projects $ ruby23 -e "hash = {}; hash[:foo] = ->(a, b) { a || b }; loop { t = Time.now; i = 0; while i < 10_000_000; i+=1; hash.fetch(:foo).call 1, 2; hash.fetch(:foo).call 1, 2; hash.fetch(:foo).call 1, 2; end; puts Time.now - t }"
4.361043
4.275673
4.263372
4.296152
^C-e:1:in `block in <main>': Interrupt
    from -e:1:in `block in <main>'
    from -e:1:in `loop'
    from -e:1:in `<main>'

It's unfortunate that MRI does not optimize this well. At least we know we're significantly faster than them regardless of how you implement this...except for refinements, which break many of our optimizations.

@schneems
Copy link
Member Author

Looking at the comment #383 (comment) it looks like the compare_by_identity.rehash gives us a pretty big speed boost on all Ruby implementations and requires minimal refactoring. Refactoring is still faster on MRI by about 16% which would be a nice speed bump, but not worth it if it drags JRuby perf down to the pit of despair. I wonder what MRI is doing that makes it so fast?

I tried to use method to generate callable objects, it was actually slower than the comparable proc invocation

  def self.fixnum(val, digest)
    digest << 'Fixnum'.freeze
    digest << val.to_s
  end

  def self.bignum(val, digest)
    digest << 'Bignum'.freeze
    digest << val.to_s
  end

  def self.array(val, digest)
    digest << 'Array'.freeze
    val.each do |element|
      ADD_VALUE_TO_DIGEST[element.class].call(element, digest)
    end
  end

  def self.hash(val, digest)
    digest << 'Hash'.freeze
    val.sort.each do |array|
      ADD_VALUE_TO_DIGEST[Array].call(array, digest)
    end
  end

  def self.set(val, digest)
    digest << 'Set'.freeze
    ADD_VALUE_TO_DIGEST[Array].call(val, digest)
  end

  def self.encoding(val, digest)
    digest << 'Encoding'.freeze
    digest << val.name
  end

  def self.symbol(val, digest)
    digest << 'Symbol'.freeze
    digest << val.to_s
  end

  def self.string(val, digest)
    digest << val
  end

  def self.true_case(val, digest)
    digest << 'TrueClass'.freeze
  end

  def self.false_case(val, digest)
    digest << 'FalseClass'.freeze
  end

  def self.nil_case(val, digest)
    digest << 'NilClass'.freeze
  end

  ADD_VALUE_TO_DIGEST = {
    String     => method(:string),
    FalseClass => method(:false_case),
    TrueClass  => method(:true_case),
    NilClass   => method(:nil_case),
    Symbol     => method(:symbol),
    Fixnum     => method(:fixnum),
    Bignum     => method(:bignum),
    Array      => method(:array),
    Hash       => method(:hash),
    Set        => method(:set),
    Encoding   => method(:encoding),
  }.compare_by_identity.rehash

Would love some ideas on syntax for an immutable Proc (i.e. anonymous function without scope lookup) if you've got them https://bugs.ruby-lang.org/issues/12901. That would help a bit here.

@schneems
Copy link
Member Author

Closing in favor of #439

@schneems schneems closed this Dec 12, 2016
@jeremy jeremy deleted the schneems/faster-digest-refinements branch January 24, 2017 19:43
This was referenced Nov 14, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants