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 BaseParser#unnormalize method to replace "\r\n" with "\n" only when "\r\n" is included #160

Merged
merged 1 commit into from
Jun 26, 2024

Conversation

naitoh
Copy link
Contributor

@naitoh naitoh commented Jun 25, 2024

Why?

See: #158 (comment)

Benchmark

RUBYLIB= BUNDLER_ORIG_RUBYLIB= /Users/naitoh/.rbenv/versions/3.3.3/bin/ruby -v -S benchmark-driver /Users/naitoh/ghq/github.com/naitoh/rexml/benchmark/parse.yaml
ruby 3.3.3 (2024-06-12 revision f1c7b6f435) [arm64-darwin22]
Calculating -------------------------------------
                         before       after  before(YJIT)  after(YJIT)
                 dom     17.674      17.567        32.759       32.316 i/s -     100.000 times in 5.657973s 5.692371s 3.052595s 3.094448s
                 sax     25.261      25.377        48.889       49.911 i/s -     100.000 times in 3.958626s 3.940640s 2.045460s 2.003575s
                pull     28.968      29.121        61.584       61.774 i/s -     100.000 times in 3.452132s 3.433967s 1.623789s 1.618809s
              stream     28.395      28.803        55.289       57.970 i/s -     100.000 times in 3.521761s 3.471812s 1.808673s 1.725029s

Comparison:
                              dom
        before(YJIT):        32.8 i/s
         after(YJIT):        32.3 i/s - 1.01x  slower
              before:        17.7 i/s - 1.85x  slower
               after:        17.6 i/s - 1.86x  slower

                              sax
         after(YJIT):        49.9 i/s
        before(YJIT):        48.9 i/s - 1.02x  slower
               after:        25.4 i/s - 1.97x  slower
              before:        25.3 i/s - 1.98x  slower

                             pull
         after(YJIT):        61.8 i/s
        before(YJIT):        61.6 i/s - 1.00x  slower
               after:        29.1 i/s - 2.12x  slower
              before:        29.0 i/s - 2.13x  slower

                           stream
         after(YJIT):        58.0 i/s
        before(YJIT):        55.3 i/s - 1.05x  slower
               after:        28.8 i/s - 2.01x  slower
              before:        28.4 i/s - 2.04x  slower

  • YJIT=ON : 0.98x - 1.05x faster
  • YJIT=OFF : 0.98x - 1.02x faster

test/test_pullparser.rb Outdated Show resolved Hide resolved
@naitoh naitoh force-pushed the optimize_base_parser_unnormalize_update branch from 97e90ce to 8261c3f Compare June 25, 2024 21:52
@naitoh naitoh requested a review from kou June 25, 2024 21:57
test/test_pullparser.rb Outdated Show resolved Hide resolved
test/test_pullparser.rb Outdated Show resolved Hide resolved
…ly when "\r\n" is included

## Why?

See: ruby#158 (comment)

## Benchmark

```
RUBYLIB= BUNDLER_ORIG_RUBYLIB= /Users/naitoh/.rbenv/versions/3.3.3/bin/ruby -v -S benchmark-driver /Users/naitoh/ghq/github.com/naitoh/rexml/benchmark/parse.yaml
ruby 3.3.3 (2024-06-12 revision f1c7b6f435) [arm64-darwin22]
Calculating -------------------------------------
                         before       after  before(YJIT)  after(YJIT)
                 dom     17.674      17.567        32.759       32.316 i/s -     100.000 times in 5.657973s 5.692371s 3.052595s 3.094448s
                 sax     25.261      25.377        48.889       49.911 i/s -     100.000 times in 3.958626s 3.940640s 2.045460s 2.003575s
                pull     28.968      29.121        61.584       61.774 i/s -     100.000 times in 3.452132s 3.433967s 1.623789s 1.618809s
              stream     28.395      28.803        55.289       57.970 i/s -     100.000 times in 3.521761s 3.471812s 1.808673s 1.725029s

Comparison:
                              dom
        before(YJIT):        32.8 i/s
         after(YJIT):        32.3 i/s - 1.01x  slower
              before:        17.7 i/s - 1.85x  slower
               after:        17.6 i/s - 1.86x  slower

                              sax
         after(YJIT):        49.9 i/s
        before(YJIT):        48.9 i/s - 1.02x  slower
               after:        25.4 i/s - 1.97x  slower
              before:        25.3 i/s - 1.98x  slower

                             pull
         after(YJIT):        61.8 i/s
        before(YJIT):        61.6 i/s - 1.00x  slower
               after:        29.1 i/s - 2.12x  slower
              before:        29.0 i/s - 2.13x  slower

                           stream
         after(YJIT):        58.0 i/s
        before(YJIT):        55.3 i/s - 1.05x  slower
               after:        28.8 i/s - 2.01x  slower
              before:        28.4 i/s - 2.04x  slower

```

- YJIT=ON : 0.98x - 1.05x faster
- YJIT=OFF : 0.98x - 1.02x faster

---------

Co-authored-by: Sutou Kouhei <kou@clear-code.com>
@naitoh naitoh force-pushed the optimize_base_parser_unnormalize_update branch from 8261c3f to 52a255b Compare June 26, 2024 14:04
@naitoh naitoh requested a review from kou June 26, 2024 14:05
@kou kou merged commit face9dd into ruby:master Jun 26, 2024
61 checks passed
@kou
Copy link
Member

kou commented Jun 26, 2024

Thanks.

@naitoh naitoh deleted the optimize_base_parser_unnormalize_update branch June 26, 2024 22:03
naitoh added a commit to naitoh/rexml that referenced this pull request Jul 4, 2024
## Why?

XML with multiple root elements is invalid.

See: ruby#160 (comment)
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