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

Rewrite Array#each in Ruby #6687

Closed
wants to merge 2 commits into from
Closed

Conversation

k0kubun
Copy link
Member

@k0kubun k0kubun commented Nov 8, 2022

Similarly to Integer#times #8388, this PR rewrites Array#each in Ruby.

microbenchmark

Interpreter

It's a bit slower than the original C method.

$ benchmark-driver benchmark/loop_each.yml -v --chruby 'before;after'
before: ruby 3.4.0dev (2024-01-13T00:28:26Z master f7178045bb) [x86_64-linux]
after: ruby 3.4.0dev (2024-01-13T04:31:50Z builtin-array-each 18c9a45314) [x86_64-linux]
Warming up --------------------------------------
           loop_each      2.129 i/s -       3.000 times in 1.408852s (469.62ms/i)
Calculating -------------------------------------
                         before       after
           loop_each      2.103       1.545 i/s -       6.000 times in 2.852494s 3.882698s

Comparison:
                        loop_each
              before:         2.1 i/s
               after:         1.5 i/s - 1.36x  slower

YJIT

It's 5.3x faster.

$ benchmark-driver benchmark/loop_each.yml -v --chruby 'before::before --yjit-call-threshold=1;after::after --yjit-call-threshold=1'
before: ruby 3.4.0dev (2024-01-13T00:28:26Z master f7178045bb) +YJIT [x86_64-linux]
after: ruby 3.4.0dev (2024-01-13T04:31:50Z builtin-array-each 18c9a45314) +YJIT [x86_64-linux]
Warming up --------------------------------------
           loop_each      2.451 i/s -       3.000 times in 1.223915s (407.97ms/i)
Calculating -------------------------------------
                         before       after
           loop_each      2.456      13.074 i/s -       7.000 times in 2.850521s 0.535404s

Comparison:
                        loop_each
               after:        13.1 i/s
              before:         2.5 i/s - 5.32x  slower

array.rb Show resolved Hide resolved
enum.c Outdated Show resolved Hide resolved
array.rb Show resolved Hide resolved
array.rb Show resolved Hide resolved
@k0kubun k0kubun force-pushed the builtin-array-each branch 3 times, most recently from 4b3a2a4 to 5f8ff7b Compare January 12, 2024 22:48
@k0kubun
Copy link
Member Author

k0kubun commented Jan 18, 2024

Given https://bugs.ruby-lang.org/issues/20182#note-5, #9533 will take this over.

@k0kubun k0kubun closed this Jan 18, 2024
@k0kubun k0kubun deleted the builtin-array-each branch January 18, 2024 21:39
@@ -373,7 +373,7 @@ def add_ivars

Fiber.new {
$ary = OBJ_COUNT.times.map { Foo.new }
$ary.each(&:add_ivars)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@k0kubun forgive me my ignorance, but just out of curiosity: why is there a change in tests from each to reverse_each?.. If this PR is a refactoring of internal mechanism, shouldn't tests remain the same, to ensure (non-)regression integrity?..

Copy link
Member Author

@k0kubun k0kubun Feb 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change of this line was actually not necessary, so I reverted it in the end #9533 (comment).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
6 participants