Skip to content

Commit

Permalink
Improve Element#attribute implementation as 6500x faster (#146)
Browse files Browse the repository at this point in the history
`Element#namespaces` is heavy method because this method needs to
traverse all ancestors of the element. `Element#attribute` calls
`namespaces` redundantly, so it is much slower.

This PR reduces `namespaces` calls in `Element#attribute`. Also, this PR
removes a redundant `respond_to?` because `namespaces` must return
`Hash` in the current implementation.

Below is the result of a benchmark for this on my laptop.

```
RUBYLIB= BUNDLER_ORIG_RUBYLIB= /Users/makenowjust/Projects/github.com/makenowjust/simple-dotfiles/.asdf/installs/ruby/3.3.2/bin/ruby -v -S benchmark-driver /Users/makenowjust/Projects/github.com/ruby/rexml/benchmark/attribute.yaml
ruby 3.3.2 (2024-05-30 revision e5a195edf6) [arm64-darwin23]
Calculating -------------------------------------
                     rexml 3.2.6      master  3.2.6(YJIT)  master(YJIT)
   attribute_with_ns     425.420     849.271       5.336k       10.629k i/s -      1.000k times in 2.350620s 1.177481s 0.187416s 0.094084s
attribute_without_ns     834.750      5.587M      10.656k        2.950M i/s -      1.000k times in 1.197963s 0.000179s 0.093846s 0.000339s

Comparison:
                attribute_with_ns
        master(YJIT):     10628.8 i/s
         3.2.6(YJIT):      5335.7 i/s - 1.99x  slower
              master:       849.3 i/s - 12.52x  slower
         rexml 3.2.6:       425.4 i/s - 24.98x  slower

             attribute_without_ns
              master:   5586593.2 i/s
        master(YJIT):   2949854.4 i/s - 1.89x  slower
         3.2.6(YJIT):     10655.8 i/s - 524.28x  slower
         rexml 3.2.6:       834.8 i/s - 6692.53x  slower

```

This result shows that `Element#attribute` is now 6500x faster than the
old implementation if `namespace` is not supplied.

It seems strange that it is slower when YJIT is enabled, but we believe
this is a separate issue.

Thank you.

---------

Co-authored-by: Sutou Kouhei <kou@cozmixng.org>
  • Loading branch information
makenowjust and kou authored Jun 13, 2024
1 parent b5bf109 commit 3b026f8
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 7 deletions.
38 changes: 38 additions & 0 deletions benchmark/attribute.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
loop_count: 1000
contexts:
- gems:
rexml: 3.2.6
require: false
prelude: require 'rexml'
- name: master
prelude: |
$LOAD_PATH.unshift(File.expand_path("lib"))
require 'rexml'
- name: 3.2.6(YJIT)
gems:
rexml: 3.2.6
require: false
prelude: |
require 'rexml'
RubyVM::YJIT.enable
- name: master(YJIT)
prelude: |
$LOAD_PATH.unshift(File.expand_path("lib"))
require 'rexml'
RubyVM::YJIT.enable
prelude: |
require 'rexml/document'
xml_source = "<deepest x:with_ns='foo' without_ns='bar'></deepest>"
100.times do
xml_source = "<nest>#{xml_source}</nest>"
end
xml_source = "<root xmlns:x='xyz'>#{xml_source}</root>"
document = REXML::Document.new(xml_source)
deepest_node = document.elements["//deepest"]
benchmark:
with_ns: deepest_node.attribute("with_ns", "xyz")
without_ns: deepest_node.attribute("without_ns")
9 changes: 2 additions & 7 deletions lib/rexml/element.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1276,16 +1276,11 @@ def [](name_or_index)
# document.root.attribute("x", "a") # => a:x='a:x'
#
def attribute( name, namespace=nil )
prefix = nil
if namespaces.respond_to? :key
prefix = namespaces.key(namespace) if namespace
else
prefix = namespaces.index(namespace) if namespace
end
prefix = namespaces.key(namespace) if namespace
prefix = nil if prefix == 'xmlns'

ret_val =
attributes.get_attribute( "#{prefix ? prefix + ':' : ''}#{name}" )
attributes.get_attribute( prefix ? "#{prefix}:#{name}" : name )

return ret_val unless ret_val.nil?
return nil if prefix.nil?
Expand Down

0 comments on commit 3b026f8

Please sign in to comment.