Bug #11461
closedRemove backtracing cleaning on Delegator methods
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
Updated by matz (Yukihiro Matsumoto) about 10 years ago
- Assignee changed from matz (Yukihiro Matsumoto) to zzak (zzak _)
OK, I accept.
Matz.
Updated by zzak (zzak _) about 10 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.655Mdefault 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.460Mdefault raising 623.950k (± 9.7%) i/s - 3.120M
Benchmark:
require 'delegate'
require 'benchmark/ips'class Foo
def name
'foo'
enddef bla raise endend
class Bar < DelegateClass(Foo)
endbar = Bar.new(Foo.new)
Benchmark.ips do |b|
b.report('default') { bar.name }
b.report('default raising') { bar.bla rescue nil }
end