Skip to content

Remove spinlock in dtoa's Balloc, just use malloc #12991

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Apr 7, 2025
Merged

Remove spinlock in dtoa's Balloc, just use malloc #12991

merged 1 commit into from
Apr 7, 2025

Conversation

jhawthorn
Copy link
Member

@jhawthorn jhawthorn commented Mar 27, 2025 •
edited
Loading

The spinlock here performs poorly when there are multiple Ractors. The difference on single threaded performance doesn't seem significant, so I think we should just use malloc.

This is under glibc malloc, others may perform differently.

make benchmark ITEM=float_to_s

compare-ruby: ruby 3.4.1 (2024-12-25 revision 48d4efcb85) +PRISM [x86_64-linux]
built-ruby: ruby 3.5.0dev (2025-03-27T01:23:04Z dtoa_just_malloc 3fc8fadbfc) +PRISM [x86_64-linux]
# Iteration per second (i/s)

|      |compare-ruby|built-ruby|
|:-----|-----------:|---------:|
|to_s  |     193.243|   200.464|
|      |           -|     1.04x|
# Iteration per second (i/s)

|                   |compare-ruby|built-ruby|
|:------------------|-----------:|---------:|
|ractor_float_to_s  |     583.808|    1.695k|
|                   |           -|     2.90x|

Format string with many %f

❯ time  ./miniruby --disable-gems -e 'a = Array.new(100) { rand }; f = "%f"*100; 100_000.times { f % a }'
./miniruby --disable-gems -e   1.89s user 0.07s system 99% cpu 1.975 total

❯ time  ruby --disable-gems -e 'a = Array.new(100) { rand }; f = "%f"*100; 100_000.times { f % a }'
ruby --disable-gems -e   2.01s user 0.02s system 99% cpu 2.044 total

rand.to_s

hyperfine "./miniruby -e '300_000.times { rand.to_s }'" "ruby --disable-gems -e '300_000.times { rand.to_s }'"
Benchmark 1: ./miniruby -e '300_000.times { rand.to_s }'
  Time (mean ± σ):     263.0 ms ±   1.4 ms    [User: 247.8 ms, System: 13.2 ms]
  Range (min … max):   260.9 ms … 266.1 ms    11 runs

Benchmark 2: ruby --disable-gems -e '300_000.times { rand.to_s }'
  Time (mean ± σ):     270.1 ms ±   3.3 ms    [User: 259.5 ms, System: 8.8 ms]
  Range (min … max):   266.0 ms … 279.3 ms    11 runs

Summary
  ./miniruby -e '300_000.times { rand.to_s }' ran
    1.03 ± 0.01 times faster than ruby --disable-gems -e '300_000.times { rand.to_s }'

Multiple Ractors

❯ make -j miniruby && hyperfine "./miniruby -e '4.times.map { Ractor.new { 300_000.times { rand.to_s } }}.each(&:take)'" "ruby --disable-gems -e '4.times.map { Ractor.new { 300_000.times { rand.to_s } }}.each(&:take)'"
Benchmark 1: ./miniruby -e '4.times.map { Ractor.new { 300_000.times { rand.to_s } }}.each(&:take)'
  Time (mean ± σ):     447.4 ms ±   7.0 ms    [User: 1417.8 ms, System: 62.4 ms]
  Range (min … max):   435.2 ms … 459.5 ms    10 runs

Benchmark 2: ruby --disable-gems -e '4.times.map { Ractor.new { 300_000.times { rand.to_s } }}.each(&:take)'
  Time (mean ± σ):      6.757 s ±  0.175 s    [User: 26.264 s, System: 0.079 s]
  Range (min … max):    6.548 s …  7.140 s    10 runs

Summary
  ./miniruby -e '4.times.map { Ractor.new { 300_000.times { rand.to_s } }}.each(&:take)' ran
   15.10 ± 0.46 times faster than ruby --disable-gems -e '4.times.map { Ractor.new { 300_000.times { rand.to_s } }}.each(&:take)'

@jhawthorn
Remove spinlock in dtoa's Balloc, use xmalloc
3fc8fad 
The spinlock here performs poorly when there are multiple Ractors. The
improvement on single threaded performance doesn't seem that
significant, so I think we should just use malloc.
@jhawthorn jhawthorn changed the title Remove spinlock in dtoa's Balloc, just use xmalloc Remove spinlock in dtoa's Balloc, just use malloc Apr 7, 2025
@jhawthorn jhawthorn merged commit b3b1d7b into ruby:master Apr 7, 2025
76 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tenderlove tenderlove tenderlove approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jhawthorn @tenderlove