Skip to content

Commit

Permalink
Change attribute.has_key?(name) to attributes[name]. (#121)
Browse files Browse the repository at this point in the history
## 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
  • Loading branch information
naitoh authored Mar 22, 2024
1 parent 0496940 commit 030bfb4
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion lib/rexml/parsers/baseparser.rb
Original file line number Diff line number Diff line change
Expand Up @@ -656,7 +656,7 @@ def parse_attributes(prefixes, curr_ns)
prefixes << prefix unless prefix == "xml"
end

if attributes.has_key?(name)
if attributes[name]
msg = "Duplicate attribute #{name.inspect}"
raise REXML::ParseException.new(msg, @source, self)
end
Expand Down

0 comments on commit 030bfb4

Please sign in to comment.