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

Optimize Source#read_until method #135

Merged
merged 1 commit into from
Jun 3, 2024

Conversation

naitoh
Copy link
Contributor

@naitoh naitoh commented Jun 2, 2024

Optimize Source#read_until method.

Benchmark

RUBYLIB= BUNDLER_ORIG_RUBYLIB= /Users/naitoh/.rbenv/versions/3.3.0/bin/ruby -v -S benchmark-driver /Users/naitoh/ghq/github.com/naitoh/rexml/benchmark/parse.yaml
ruby 3.3.0 (2023-12-25 revision 5124f9ac75) [arm64-darwin22]
Calculating -------------------------------------
                         before       after  before(YJIT)  after(YJIT)
                 dom      9.877       9.992        15.605       17.559 i/s -     100.000 times in 10.124592s 10.008017s 6.408031s 5.695167s
                 sax     22.903      25.151        39.482       50.846 i/s -     100.000 times in 4.366300s 3.975922s 2.532822s 1.966706s
                pull     25.940      30.474        44.685       61.450 i/s -     100.000 times in 3.855070s 3.281511s 2.237879s 1.627346s
              stream     25.255      29.500        41.819       53.605 i/s -     100.000 times in 3.959539s 3.389825s 2.391256s 1.865505s

Comparison:
                              dom
         after(YJIT):        17.6 i/s
        before(YJIT):        15.6 i/s - 1.13x  slower
               after:        10.0 i/s - 1.76x  slower
              before:         9.9 i/s - 1.78x  slower

                              sax
         after(YJIT):        50.8 i/s
        before(YJIT):        39.5 i/s - 1.29x  slower
               after:        25.2 i/s - 2.02x  slower
              before:        22.9 i/s - 2.22x  slower

                             pull
         after(YJIT):        61.4 i/s
        before(YJIT):        44.7 i/s - 1.38x  slower
               after:        30.5 i/s - 2.02x  slower
              before:        25.9 i/s - 2.37x  slower

                           stream
         after(YJIT):        53.6 i/s
        before(YJIT):        41.8 i/s - 1.28x  slower
               after:        29.5 i/s - 1.82x  slower
              before:        25.3 i/s - 2.12x  slower

  • YJIT=ON : 1.13x - 1.38x faster
  • YJIT=OFF : 1.01x - 1.17x faster

lib/rexml/source.rb Outdated Show resolved Hide resolved
@kou kou changed the title Optimized Source#read_until method Optimize Source#read_until method Jun 2, 2024
@kou
Copy link
Member

kou commented Jun 2, 2024

BTW, can we remove .elements.each("root/child") {|_|} from

'dom' : REXML::Document.new(xml).elements.each("root/child") {|_|}
?

I think that "dom" is slower than others because of it. And .elements.each("root/child") {|_|} isn't parse processing.

@naitoh naitoh force-pushed the optimized_read_until_method branch from 30017cc to 875f4c6 Compare June 2, 2024 21:28
## Benchmark
```
RUBYLIB= BUNDLER_ORIG_RUBYLIB= /Users/naitoh/.rbenv/versions/3.3.0/bin/ruby -v -S benchmark-driver /Users/naitoh/ghq/github.com/naitoh/rexml/benchmark/parse.yaml
ruby 3.3.0 (2023-12-25 revision 5124f9ac75) [arm64-darwin22]
Calculating -------------------------------------
                         before       after  before(YJIT)  after(YJIT)
                 dom      9.877       9.992        15.605       17.559 i/s -     100.000 times in 10.124592s 10.008017s 6.408031s 5.695167s
                 sax     22.903      25.151        39.482       50.846 i/s -     100.000 times in 4.366300s 3.975922s 2.532822s 1.966706s
                pull     25.940      30.474        44.685       61.450 i/s -     100.000 times in 3.855070s 3.281511s 2.237879s 1.627346s
              stream     25.255      29.500        41.819       53.605 i/s -     100.000 times in 3.959539s 3.389825s 2.391256s 1.865505s

Comparison:
                              dom
         after(YJIT):        17.6 i/s
        before(YJIT):        15.6 i/s - 1.13x  slower
               after:        10.0 i/s - 1.76x  slower
              before:         9.9 i/s - 1.78x  slower

                              sax
         after(YJIT):        50.8 i/s
        before(YJIT):        39.5 i/s - 1.29x  slower
               after:        25.2 i/s - 2.02x  slower
              before:        22.9 i/s - 2.22x  slower

                             pull
         after(YJIT):        61.4 i/s
        before(YJIT):        44.7 i/s - 1.38x  slower
               after:        30.5 i/s - 2.02x  slower
              before:        25.9 i/s - 2.37x  slower

                           stream
         after(YJIT):        53.6 i/s
        before(YJIT):        41.8 i/s - 1.28x  slower
               after:        29.5 i/s - 1.82x  slower
              before:        25.3 i/s - 2.12x  slower

```

- YJIT=ON : 1.13x - 1.38x faster
- YJIT=OFF : 1.01x - 1.17x faster

Co-authored-by: Sutou Kouhei <kou@clear-code.com>
@naitoh naitoh force-pushed the optimized_read_until_method branch from 875f4c6 to e679cd4 Compare June 2, 2024 21:44
@naitoh
Copy link
Contributor Author

naitoh commented Jun 2, 2024

BTW, can we remove .elements.each("root/child") {|_|} from

'dom' : REXML::Document.new(xml).elements.each("root/child") {|_|}

?

I think that "dom" is slower than others because of it. And .elements.each("root/child") {|_|} isn't parse processing.

@kou

I agree.
I created #136.
Thanks!

@naitoh naitoh requested a review from kou June 2, 2024 21:50
@kou kou merged commit 037c16a into ruby:master Jun 3, 2024
55 checks passed
@kou
Copy link
Member

kou commented Jun 3, 2024

Thanks!

@naitoh naitoh deleted the optimized_read_until_method branch June 3, 2024 01:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants