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 IOSource#read_until method #210

Merged
merged 3 commits into from
Oct 9, 2024
Merged

Conversation

naitoh
Copy link
Contributor

@naitoh naitoh commented Oct 6, 2024

Why?

The result of encode(term) can be cached.

Benchmark

RUBYLIB= BUNDLER_ORIG_RUBYLIB= /Users/naitoh/.rbenv/versions/3.3.4/bin/ruby -v -S benchmark-driver /Users/naitoh/ghq/github.com/naitoh/rexml/benchmark/parse.yaml
ruby 3.3.4 (2024-07-09 revision be1089c8ec) [arm64-darwin22]
Calculating -------------------------------------
                         before       after  before(YJIT)  after(YJIT)
                 dom     17.546      18.512        32.282       32.306 i/s -     100.000 times in 5.699323s 5.402026s 3.097658s 3.095448s
                 sax     25.435      28.294        47.526       50.074 i/s -     100.000 times in 3.931613s 3.534310s 2.104122s 1.997057s
                pull     29.471      31.870        54.400       57.554 i/s -     100.000 times in 3.393211s 3.137793s 1.838222s 1.737494s
              stream     29.169      31.153        51.613       52.898 i/s -     100.000 times in 3.428318s 3.209941s 1.937508s 1.890424s

Comparison:
                              dom
         after(YJIT):        32.3 i/s
        before(YJIT):        32.3 i/s - 1.00x  slower
               after:        18.5 i/s - 1.75x  slower
              before:        17.5 i/s - 1.84x  slower

                              sax
         after(YJIT):        50.1 i/s
        before(YJIT):        47.5 i/s - 1.05x  slower
               after:        28.3 i/s - 1.77x  slower
              before:        25.4 i/s - 1.97x  slower

                             pull
         after(YJIT):        57.6 i/s
        before(YJIT):        54.4 i/s - 1.06x  slower
               after:        31.9 i/s - 1.81x  slower
              before:        29.5 i/s - 1.95x  slower

                           stream
         after(YJIT):        52.9 i/s
        before(YJIT):        51.6 i/s - 1.02x  slower
               after:        31.2 i/s - 1.70x  slower
              before:        29.2 i/s - 1.81x  slower

  • YJIT=ON : 1.00x - 1.06x faster
  • YJIT=OFF : 1.05x - 1.11x faster

@naitoh naitoh marked this pull request as ready for review October 6, 2024 23:29
pattern = Private::PRE_DEFINED_TERM_PATTERNS[term]
if pattern.nil?
pattern = /#{Regexp.escape(term)}/
term = encode(term)
Copy link
Member

Choose a reason for hiding this comment

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

Does this work when encoding is UTF-16 or UTF-32?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry.
I have confirmed that there is a problem with UTF-16 and will change it to draft.

@naitoh naitoh marked this pull request as draft October 8, 2024 00:22
naitoh added 2 commits October 9, 2024 09:15
## Why?
The result of `encode(term)` can be cached.

## Benchmark

```
RUBYLIB= BUNDLER_ORIG_RUBYLIB= /Users/naitoh/.rbenv/versions/3.3.4/bin/ruby -v -S benchmark-driver /Users/naitoh/ghq/github.com/naitoh/rexml/benchmark/parse.yaml
ruby 3.3.4 (2024-07-09 revision be1089c8ec) [arm64-darwin22]
Calculating -------------------------------------
                         before       after  before(YJIT)  after(YJIT)
                 dom     17.546      18.512        32.282       32.306 i/s -     100.000 times in 5.699323s 5.402026s 3.097658s 3.095448s
                 sax     25.435      28.294        47.526       50.074 i/s -     100.000 times in 3.931613s 3.534310s 2.104122s 1.997057s
                pull     29.471      31.870        54.400       57.554 i/s -     100.000 times in 3.393211s 3.137793s 1.838222s 1.737494s
              stream     29.169      31.153        51.613       52.898 i/s -     100.000 times in 3.428318s 3.209941s 1.937508s 1.890424s

Comparison:
                              dom
         after(YJIT):        32.3 i/s
        before(YJIT):        32.3 i/s - 1.00x  slower
               after:        18.5 i/s - 1.75x  slower
              before:        17.5 i/s - 1.84x  slower

                              sax
         after(YJIT):        50.1 i/s
        before(YJIT):        47.5 i/s - 1.05x  slower
               after:        28.3 i/s - 1.77x  slower
              before:        25.4 i/s - 1.97x  slower

                             pull
         after(YJIT):        57.6 i/s
        before(YJIT):        54.4 i/s - 1.06x  slower
               after:        31.9 i/s - 1.81x  slower
              before:        29.5 i/s - 1.95x  slower

                           stream
         after(YJIT):        52.9 i/s
        before(YJIT):        51.6 i/s - 1.02x  slower
               after:        31.2 i/s - 1.70x  slower
              before:        29.2 i/s - 1.81x  slower

```

- YJIT=ON : 1.00x - 1.06x faster
- YJIT=OFF : 1.05x - 1.11x faster
@naitoh
Copy link
Contributor Author

naitoh commented Oct 9, 2024

@naitoh naitoh requested a review from kou October 9, 2024 00:46
@naitoh naitoh marked this pull request as ready for review October 9, 2024 00:46
test/test_document.rb Outdated Show resolved Hide resolved
@kou kou merged commit 1d0c362 into ruby:master Oct 9, 2024
14 checks passed
@kou
Copy link
Member

kou commented Oct 9, 2024

Thanks.

@naitoh naitoh deleted the read_until_term branch October 9, 2024 01:50
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