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

Change attribute.has_key?(name) to attributes[name]. #121

Merged
merged 1 commit into from
Mar 22, 2024

Conversation

naitoh
Copy link
Contributor

@naitoh naitoh commented Mar 20, 2024

Why?

attributes[name] is faster than attribute.has_key?(name) in Micro Benchmark.

However, the Benchmark did not show a significant difference.
Would like to merge if possible, how about it?

See: #119 (comment)

Micro Benchmark

$ cat benchmark/attributes.yaml
loop_count: 100000
contexts:
  - name: No YJIT
    prelude: |
      $LOAD_PATH.unshift(File.expand_path("lib"))
      require 'rexml'
  - name: YJIT
    prelude: |
      $LOAD_PATH.unshift(File.expand_path("lib"))
      require 'rexml'
      RubyVM::YJIT.enable

prelude: |
  attributes = {}
  name = :a

benchmark:
  'attributes[name]' : attributes[name]
  'attributes.has_key?(name)' : attributes.has_key?(name)
$ benchmark-driver benchmark/attributes.yaml
Calculating -------------------------------------
                             No YJIT        YJIT
         attributes[name]    53.362M     53.562M i/s -    100.000k times in 0.001874s 0.001867s
attributes.has_key?(name)    45.025M     45.005M i/s -    100.000k times in 0.002221s 0.002222s

Comparison:
                      attributes[name]
                     YJIT:  53561863.6 i/s
                  No YJIT:  53361791.1 i/s - 1.00x  slower

             attributes.has_key?(name)
                  No YJIT:  45024765.3 i/s
                     YJIT:  45004502.0 i/s - 1.00x  slower

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     10.786      10.783        18.196       17.959 i/s -     100.000 times in 9.270908s 9.273657s 5.495854s 5.568326s
                 sax     30.213      30.430        57.030       56.672 i/s -     100.000 times in 3.309845s 3.286240s 1.753459s 1.764551s
                pull     35.211      35.259        70.817       70.784 i/s -     100.000 times in 2.840056s 2.836136s 1.412098s 1.412754s
              stream     34.281      34.475        63.084       62.978 i/s -     100.000 times in 2.917067s 2.900689s 1.585196s 1.587860s

Comparison:
                              dom
        before(YJIT):        18.2 i/s
         after(YJIT):        18.0 i/s - 1.01x  slower
              before:        10.8 i/s - 1.69x  slower
               after:        10.8 i/s - 1.69x  slower

                              sax
        before(YJIT):        57.0 i/s
         after(YJIT):        56.7 i/s - 1.01x  slower
               after:        30.4 i/s - 1.87x  slower
              before:        30.2 i/s - 1.89x  slower

                             pull
        before(YJIT):        70.8 i/s
         after(YJIT):        70.8 i/s - 1.00x  slower
               after:        35.3 i/s - 2.01x  slower
              before:        35.2 i/s - 2.01x  slower

                           stream
        before(YJIT):        63.1 i/s
         after(YJIT):        63.0 i/s - 1.00x  slower
               after:        34.5 i/s - 1.83x  slower
              before:        34.3 i/s - 1.84x  slower

  • YJIT=ON : 0.98x - 1.00x faster
  • YJIT=OFF : 1.00x - 1.00x faster

## Why?
`attributes[name]` is faster than `attribute.has_key?(name)` in Micro Benchmark.

However, the Benchmark did not show a significant difference.
Would like to merge if possible, how about it?

See: ruby#119 (comment)

## Micro Benchmark

```
$ cat benchmark/attributes.yaml
loop_count: 100000
contexts:
  - name: No YJIT
    prelude: |
      $LOAD_PATH.unshift(File.expand_path("lib"))
      require 'rexml'
  - name: YJIT
    prelude: |
      $LOAD_PATH.unshift(File.expand_path("lib"))
      require 'rexml'
      RubyVM::YJIT.enable

prelude: |
  attributes = {}
  name = :a

benchmark:
  'attributes[name]' : attributes[name]
  'attributes.has_key?(name)' : attributes.has_key?(name)
```

```
$ benchmark-driver benchmark/attributes.yaml
Calculating -------------------------------------
                             No YJIT        YJIT
         attributes[name]    53.362M     53.562M i/s -    100.000k times in 0.001874s 0.001867s
attributes.has_key?(name)    45.025M     45.005M i/s -    100.000k times in 0.002221s 0.002222s

Comparison:
                      attributes[name]
                     YJIT:  53561863.6 i/s
                  No YJIT:  53361791.1 i/s - 1.00x  slower

             attributes.has_key?(name)
                  No YJIT:  45024765.3 i/s
                     YJIT:  45004502.0 i/s - 1.00x  slower
```

## 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     10.786      10.783        18.196       17.959 i/s -     100.000 times in 9.270908s 9.273657s 5.495854s 5.568326s
                 sax     30.213      30.430        57.030       56.672 i/s -     100.000 times in 3.309845s 3.286240s 1.753459s 1.764551s
                pull     35.211      35.259        70.817       70.784 i/s -     100.000 times in 2.840056s 2.836136s 1.412098s 1.412754s
              stream     34.281      34.475        63.084       62.978 i/s -     100.000 times in 2.917067s 2.900689s 1.585196s 1.587860s

Comparison:
                              dom
        before(YJIT):        18.2 i/s
         after(YJIT):        18.0 i/s - 1.01x  slower
              before:        10.8 i/s - 1.69x  slower
               after:        10.8 i/s - 1.69x  slower

                              sax
        before(YJIT):        57.0 i/s
         after(YJIT):        56.7 i/s - 1.01x  slower
               after:        30.4 i/s - 1.87x  slower
              before:        30.2 i/s - 1.89x  slower

                             pull
        before(YJIT):        70.8 i/s
         after(YJIT):        70.8 i/s - 1.00x  slower
               after:        35.3 i/s - 2.01x  slower
              before:        35.2 i/s - 2.01x  slower

                           stream
        before(YJIT):        63.1 i/s
         after(YJIT):        63.0 i/s - 1.00x  slower
               after:        34.5 i/s - 1.83x  slower
              before:        34.3 i/s - 1.84x  slower

```

- YJIT=ON : 0.98x - 1.00x faster
- YJIT=OFF : 1.00x - 1.00x faster
@kou
Copy link
Member

kou commented Mar 22, 2024

I checked why [] is faster than has_key? by gdb.
[] is optimized by VM. [] is compiled as optimized operation(?), which is internally implemented as vm_opt_aref().

OK. [] isn't faster than has_key? in REXML but we'll use it.

@kou kou merged commit 030bfb4 into ruby:master Mar 22, 2024
38 of 40 checks passed
@naitoh naitoh deleted the attributes_name branch March 22, 2024 14:09
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