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 SAX2Parser#get_namespace #207

Merged
merged 2 commits into from
Sep 24, 2024

Conversation

naitoh
Copy link
Contributor

@naitoh naitoh commented Sep 23, 2024

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     18.085      17.677        33.086       32.778 i/s -     100.000 times in 5.529372s 5.657097s 3.022471s 3.050832s
                 sax     25.450      26.182        44.797       47.916 i/s -     100.000 times in 3.929249s 3.819475s 2.232309s 2.086982s
                pull     29.160      29.089        55.407       53.531 i/s -     100.000 times in 3.429304s 3.437757s 1.804825s 1.868072s
              stream     29.137      29.055        52.780       51.368 i/s -     100.000 times in 3.432007s 3.441754s 1.894649s 1.946724s

Comparison:
                              dom
        before(YJIT):        33.1 i/s
         after(YJIT):        32.8 i/s - 1.01x  slower
              before:        18.1 i/s - 1.83x  slower
               after:        17.7 i/s - 1.87x  slower

                              sax
         after(YJIT):        47.9 i/s
        before(YJIT):        44.8 i/s - 1.07x  slower
               after:        26.2 i/s - 1.83x  slower
              before:        25.5 i/s - 1.88x  slower

                             pull
        before(YJIT):        55.4 i/s
         after(YJIT):        53.5 i/s - 1.04x  slower
              before:        29.2 i/s - 1.90x  slower
               after:        29.1 i/s - 1.90x  slower

                           stream
        before(YJIT):        52.8 i/s
         after(YJIT):        51.4 i/s - 1.03x  slower
              before:        29.1 i/s - 1.81x  slower
               after:        29.1 i/s - 1.82x  slower
  • sax
    • YJIT=ON : 1.07x faster
    • YJIT=OFF : 1.03x faster

@naitoh naitoh marked this pull request as ready for review September 23, 2024 02:40
test/test_sax.rb Outdated
Comment on lines 113 to 115
parser.listen(:start_element){|uri, localname, qname, attrs|
elements << [uri, localname, qname, attrs]
}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
parser.listen(:start_element){|uri, localname, qname, attrs|
elements << [uri, localname, qname, attrs]
}
parser.listen(:start_element) do |uri, localname, qname, attrs|
elements << [uri, localname, qname, attrs]
end

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK.
I see.

test/test_sax.rb Outdated
Comment on lines 137 to 139
parser.listen(:start_element){|uri, localname, qname, attrs|
elements << [uri, localname, qname, attrs]
}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
parser.listen(:start_element){|uri, localname, qname, attrs|
elements << [uri, localname, qname, attrs]
}
parser.listen(:start_element) do |uri, localname, qname, attrs|
elements << [uri, localname, qname, attrs]
end

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK.
I see.

```
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     18.085      17.677        33.086       32.778 i/s -     100.000 times in 5.529372s 5.657097s 3.022471s 3.050832s
                 sax     25.450      26.182        44.797       47.916 i/s -     100.000 times in 3.929249s 3.819475s 2.232309s 2.086982s
                pull     29.160      29.089        55.407       53.531 i/s -     100.000 times in 3.429304s 3.437757s 1.804825s 1.868072s
              stream     29.137      29.055        52.780       51.368 i/s -     100.000 times in 3.432007s 3.441754s 1.894649s 1.946724s

Comparison:
                              dom
        before(YJIT):        33.1 i/s
         after(YJIT):        32.8 i/s - 1.01x  slower
              before:        18.1 i/s - 1.83x  slower
               after:        17.7 i/s - 1.87x  slower

                              sax
         after(YJIT):        47.9 i/s
        before(YJIT):        44.8 i/s - 1.07x  slower
               after:        26.2 i/s - 1.83x  slower
              before:        25.5 i/s - 1.88x  slower

                             pull
        before(YJIT):        55.4 i/s
         after(YJIT):        53.5 i/s - 1.04x  slower
              before:        29.2 i/s - 1.90x  slower
               after:        29.1 i/s - 1.90x  slower

                           stream
        before(YJIT):        52.8 i/s
         after(YJIT):        51.4 i/s - 1.03x  slower
              before:        29.1 i/s - 1.81x  slower
               after:        29.1 i/s - 1.82x  slower
```

- sax
  - YJIT=ON : 1.07x faster
  - YJIT=OFF : 1.03x faster
@naitoh naitoh force-pushed the optimize_sax2parser_get_namespace branch from 219ed7f to 372ffb8 Compare September 24, 2024 14:39
@naitoh naitoh requested a review from kou September 24, 2024 14:49
@kou kou merged commit 2e1cd64 into ruby:master Sep 24, 2024
60 of 61 checks passed
@kou
Copy link
Member

kou commented Sep 24, 2024

Thanks.

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