[#70257] [Ruby trunk - Feature #11420] [Open] Introduce ID key table into MRI — ko1@...

Issue #11420 has been reported by Koichi Sasada.

11 messages 2015/08/06

[ruby-core:70453] [Ruby trunk - Bug #11461] [Open] Remove backtracing cleaning on Delegator methods

From: rafaelmfranca@...
Date: 2015-08-19 03:13:26 UTC
List: ruby-core #70453
Issue #11461 has been reported by Rafael Fran=C3=A7a.

----------------------------------------
Bug #11461: Remove backtracing cleaning on Delegator methods
https://bugs.ruby-lang.org/issues/11461

* Author: Rafael Fran=C3=A7a
* Status: Open
* Priority: Normal
* Assignee: Yukihiro Matsumoto
* ruby -v:=20
* Backport: 2.0.0: UNKNOWN, 2.1: UNKNOWN, 2.2: UNKNOWN
----------------------------------------
This improve the performance in 18 times when the delegated method raises s=
ome exception and in 15% for the success case.

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

    require 'delegate'
    require 'benchmark/ips'

    class Foo
      def name
        'foo'
      end

      def bla
        raise
      end
    end

    class Bar < DelegateClass(Foo)
    end

    bar =3D 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 (=C2=B1 9.9%) i/s -      6.106M
         default raising     14.399k (=C2=B113.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 (=C2=B1 8.0%) i/s -      7.403M
         default raising    263.340k (=C2=B1 8.0%) i/s -      1.313M

We could expect more performance difference in a huge application since the=
ir 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 pa=
ths.

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 =3D~ t} if $@
                                      |   346  |     end

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

---Files--------------------------------
delegate.diff (1.22 KB)


--=20
https://bugs.ruby-lang.org/

In This Thread

Prev Next