Project

General

Profile

Bug #11461

Remove backtracing cleaning on Delegator methods

Added by rafaelfranca (Rafael França) almost 4 years ago. Updated over 3 years ago.

Status:
Closed
Priority:
Normal
Target version:
-
[ruby-core:<unknown>]

Description

This improve the performance in 18 times when the delegated method raises some exception and in 15% for the success case.

Here a benchmark result with a simpler script: (The backtrace have less than 10 items)

require 'delegate'
require 'benchmark/ips'

class Foo
  def name
    'foo'
  end

  def bla
    raise
  end
end

class Bar < DelegateClass(Foo)
end

bar = Bar.new(Foo.new)

Benchmark.ips do |b|
  b.report('default') { bar.name }
  b.report('default raising') { bar.bla rescue nil }
end

Before this patch:

$ ruby delegator_test.rb
Calculating -------------------------------------
             default    42.999k i/100ms
     default raising     1.456k i/100ms
-------------------------------------------------
             default      1.232M (± 9.9%) i/s -      6.106M
     default raising     14.399k (±13.6%) i/s -     71.344k

With this patch:

$ ruby delegator_test.rb
Calculating -------------------------------------
             default    49.024k i/100ms
     default raising    19.308k i/100ms
-------------------------------------------------
             default      1.484M (± 8.0%) i/s -      7.403M
     default raising    263.340k (± 8.0%) i/s -      1.313M

We could expect more performance difference in a huge application since their backtracers are bigger.

This changes the output of the exception from:

delegator_test.rb:18:in `bla': unhandled exception
    from delegator_test.rb:43:in `<main>'

To:

delegator_test.rb:18:in `bla': unhandled exception
    from /opt/ruby/lib/delegate.rb:340:in `block in delegating_block'
    from delegator_test.rb:43:in `<main>'

This patch is important because Rails 4.2 use Delegator in some critical paths.

From our production application this backtrace cleaner was one of the top 3 most CPU consuming calls. Here is the output of stackprof.

block (2 levels) in Delegator.delegating_block (/usr/lib/shopify-ruby/2.1.6-shopify2/lib/ruby/2.1.0/delegate.rb:345)
  samples:  1228 self (5.2%)  /   1228 total (5.2%)
  callers:
    1228  (  100.0%)  block in Delegator.delegating_block
  code:
 1228    (5.2%) /  1228   (5.2%)  |   345  |       $@.delete_if {|t| /\A#{Regexp.quote(__FILE__)}:#{__LINE__-2}:/o =~ t} if $@
                                  |   346  |     end

Given the positive performance impact and that the difference of the backtrace output is not so big, I believe it is worth to consider removing the backtrace cleaner.


Files

delegate.diff (1.22 KB) delegate.diff rafaelfranca (Rafael França), 08/19/2015 03:12 AM

Associated revisions

Revision 0a4129cb
Added by zzak (Zachary Scott) over 3 years ago

  • lib/delegate.rb: Remove backtrace cleaning for delegated methods This patch was provided by Rafael França and greatly improves performance when an exception is raised. [Bug #11461]

Before:
Calculating -------------------------------------
default 86.209k i/100ms
default raising 2.209k i/100ms
-------------------------------------------------
default 1.953M (±11.0%) i/s - 9.655M
default raising 21.826k (±13.5%) i/s - 108.241k

After:
Calculating -------------------------------------
default 72.211k i/100ms
default raising 34.288k i/100ms
-------------------------------------------------
default 2.013M (±18.7%) i/s - 9.460M
default raising 623.950k (± 9.7%) i/s - 3.120M

Benchmark:
require 'delegate'
require 'benchmark/ips'

class Foo
  def name
    'foo'
  end

  def bla
    raise
  end
end

class Bar < DelegateClass(Foo)
end

bar = Bar.new(Foo.new)

Benchmark.ips do |b|
  b.report('default') { bar.name }
  b.report('default raising') { bar.bla rescue nil }
end

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@51806 b2dd03c8-39d4-4d8f-98ff-823fe69b080e

Revision 51806
Added by zzak (Zachary Scott) over 3 years ago

  • lib/delegate.rb: Remove backtrace cleaning for delegated methods This patch was provided by Rafael França and greatly improves performance when an exception is raised. [Bug #11461]

Before:
Calculating -------------------------------------
default 86.209k i/100ms
default raising 2.209k i/100ms
-------------------------------------------------
default 1.953M (±11.0%) i/s - 9.655M
default raising 21.826k (±13.5%) i/s - 108.241k

After:
Calculating -------------------------------------
default 72.211k i/100ms
default raising 34.288k i/100ms
-------------------------------------------------
default 2.013M (±18.7%) i/s - 9.460M
default raising 623.950k (± 9.7%) i/s - 3.120M

Benchmark:
require 'delegate'
require 'benchmark/ips'

class Foo
  def name
    'foo'
  end

  def bla
    raise
  end
end

class Bar < DelegateClass(Foo)
end

bar = Bar.new(Foo.new)

Benchmark.ips do |b|
  b.report('default') { bar.name }
  b.report('default raising') { bar.bla rescue nil }
end

Revision 51806
Added by zzak (Zachary Scott) over 3 years ago

  • lib/delegate.rb: Remove backtrace cleaning for delegated methods This patch was provided by Rafael França and greatly improves performance when an exception is raised. [Bug #11461]

Before:
Calculating -------------------------------------
default 86.209k i/100ms
default raising 2.209k i/100ms
-------------------------------------------------
default 1.953M (±11.0%) i/s - 9.655M
default raising 21.826k (±13.5%) i/s - 108.241k

After:
Calculating -------------------------------------
default 72.211k i/100ms
default raising 34.288k i/100ms
-------------------------------------------------
default 2.013M (±18.7%) i/s - 9.460M
default raising 623.950k (± 9.7%) i/s - 3.120M

Benchmark:
require 'delegate'
require 'benchmark/ips'

class Foo
  def name
    'foo'
  end

  def bla
    raise
  end
end

class Bar < DelegateClass(Foo)
end

bar = Bar.new(Foo.new)

Benchmark.ips do |b|
  b.report('default') { bar.name }
  b.report('default raising') { bar.bla rescue nil }
end

Revision 51806
Added by zzak (Zachary Scott) over 3 years ago

  • lib/delegate.rb: Remove backtrace cleaning for delegated methods This patch was provided by Rafael França and greatly improves performance when an exception is raised. [Bug #11461]

Before:
Calculating -------------------------------------
default 86.209k i/100ms
default raising 2.209k i/100ms
-------------------------------------------------
default 1.953M (±11.0%) i/s - 9.655M
default raising 21.826k (±13.5%) i/s - 108.241k

After:
Calculating -------------------------------------
default 72.211k i/100ms
default raising 34.288k i/100ms
-------------------------------------------------
default 2.013M (±18.7%) i/s - 9.460M
default raising 623.950k (± 9.7%) i/s - 3.120M

Benchmark:
require 'delegate'
require 'benchmark/ips'

class Foo
  def name
    'foo'
  end

  def bla
    raise
  end
end

class Bar < DelegateClass(Foo)
end

bar = Bar.new(Foo.new)

Benchmark.ips do |b|
  b.report('default') { bar.name }
  b.report('default raising') { bar.bla rescue nil }
end

Revision 51806
Added by zzak (Zachary Scott) over 3 years ago

  • lib/delegate.rb: Remove backtrace cleaning for delegated methods This patch was provided by Rafael França and greatly improves performance when an exception is raised. [Bug #11461]

Before:
Calculating -------------------------------------
default 86.209k i/100ms
default raising 2.209k i/100ms
-------------------------------------------------
default 1.953M (±11.0%) i/s - 9.655M
default raising 21.826k (±13.5%) i/s - 108.241k

After:
Calculating -------------------------------------
default 72.211k i/100ms
default raising 34.288k i/100ms
-------------------------------------------------
default 2.013M (±18.7%) i/s - 9.460M
default raising 623.950k (± 9.7%) i/s - 3.120M

Benchmark:
require 'delegate'
require 'benchmark/ips'

class Foo
  def name
    'foo'
  end

  def bla
    raise
  end
end

class Bar < DelegateClass(Foo)
end

bar = Bar.new(Foo.new)

Benchmark.ips do |b|
  b.report('default') { bar.name }
  b.report('default raising') { bar.bla rescue nil }
end

Revision 51806
Added by zzak (Zachary Scott) over 3 years ago

  • lib/delegate.rb: Remove backtrace cleaning for delegated methods This patch was provided by Rafael França and greatly improves performance when an exception is raised. [Bug #11461]

Before:
Calculating -------------------------------------
default 86.209k i/100ms
default raising 2.209k i/100ms
-------------------------------------------------
default 1.953M (±11.0%) i/s - 9.655M
default raising 21.826k (±13.5%) i/s - 108.241k

After:
Calculating -------------------------------------
default 72.211k i/100ms
default raising 34.288k i/100ms
-------------------------------------------------
default 2.013M (±18.7%) i/s - 9.460M
default raising 623.950k (± 9.7%) i/s - 3.120M

Benchmark:
require 'delegate'
require 'benchmark/ips'

class Foo
  def name
    'foo'
  end

  def bla
    raise
  end
end

class Bar < DelegateClass(Foo)
end

bar = Bar.new(Foo.new)

Benchmark.ips do |b|
  b.report('default') { bar.name }
  b.report('default raising') { bar.bla rescue nil }
end

History

#1

Updated by matz (Yukihiro Matsumoto) over 3 years ago

  • Assignee changed from matz (Yukihiro Matsumoto) to zzak (Zachary Scott)

OK, I accept.

Matz.

#2

Updated by zzak (Zachary Scott) over 3 years ago

  • Status changed from Open to Closed

Applied in changeset r51806.


  • lib/delegate.rb: Remove backtrace cleaning for delegated methods This patch was provided by Rafael França and greatly improves performance when an exception is raised. [Bug #11461]

Before:
Calculating -------------------------------------
default 86.209k i/100ms
default raising 2.209k i/100ms
-------------------------------------------------
default 1.953M (±11.0%) i/s - 9.655M
default raising 21.826k (±13.5%) i/s - 108.241k

After:
Calculating -------------------------------------
default 72.211k i/100ms
default raising 34.288k i/100ms
-------------------------------------------------
default 2.013M (±18.7%) i/s - 9.460M
default raising 623.950k (± 9.7%) i/s - 3.120M

Benchmark:
require 'delegate'
require 'benchmark/ips'

class Foo
  def name
    'foo'
  end

  def bla
    raise
  end
end

class Bar < DelegateClass(Foo)
end

bar = Bar.new(Foo.new)

Benchmark.ips do |b|
  b.report('default') { bar.name }
  b.report('default raising') { bar.bla rescue nil }
end

Also available in: Atom PDF