Change namespace cache strategy

Namespace calculation for each node can't be cached to document because document lookup is slow for deeply nested nodes.
Change namespace cache strategy, inject cached hash as an argument to retrieve namespace/namespaces from XPath match operation and also for bare namespace call.

Author: tompng

lib/rexml/doctype.rb

@@ -113,27 +113,26 @@ def node_type
     end
 
     def attributes_of element
-      attribute_declarations_of(element).map do |key, value|
+      raw_attributes = _attlist_mappings[element] || {}
+      raw_attributes.map do |key, value|
         Attribute.new(key, value)
       end
     end
 
     def attribute_of element, attribute
-      attribute_declarations_of(element)[attribute]
+      _attlist_mappings[element]&.[](attribute)
     end
 
-    def attribute_declarations_of(name)
-      raw_attributes = {}
-      decls = select do |child|
-        child.kind_of?(AttlistDecl) && child.element_name == name
-      end
-      decls.each do |child|
+    def _attlist_mappings
+      mapping = {}
+      grep(AttlistDecl).each do |child|
+        raw_attributes = mapping[child.element_name] ||= {}
         child.each do |key, val|
           # First declaration wins
           raw_attributes[key] = val unless raw_attributes.key? key
         end
       end
-      raw_attributes
+      mapping
     end
 
     def clone

lib/rexml/element.rb

@@ -588,12 +588,7 @@ def prefixes
     #   d.elements['//c'].namespaces # => {"x"=>"1", "y"=>"2", "z"=>"3"}
     #
     def namespaces
-      namespaces_cache = document&.__send__(:namespaces_cache)
-      if namespaces_cache
-        namespaces_cache[self] ||= calculate_namespaces
-      else
-        calculate_namespaces
-      end
+      _calculate_namespaces
     end
 
     # :call-seq:
@@ -618,13 +613,10 @@ def namespaces
     #
     def namespace(prefix=nil)
       if prefix.nil?
-        prefix = prefix()
+        _namespace_internal
+      else
+        _namespace_lookup_internal(prefix)
       end
-      prefix = (prefix == '') ? 'xmlns' : prefix.delete_prefix("xmlns:")
-      ns = namespaces[prefix]
-
-      ns = '' if ns.nil? and prefix == 'xmlns'
-      ns
     end
 
     # :call-seq:
@@ -1507,15 +1499,32 @@ def write(output=$stdout, indent=-1, transitive=false, ie_hack=false)
       formatter.write( self, output )
     end
 
-    private
-    def calculate_namespaces
-      if parent
-        parent.namespaces.merge(attributes.namespaces)
-      else
-        attributes.namespaces
-      end
+    # Returns namespace of the element
+    def _namespace_internal(namespaces = _calculate_namespaces) # :nodoc:
+      _namespace_lookup_internal(prefix, namespaces)
+    end
+
+    # Lookup namespace for the given prefix in the context of the element
+    def _namespace_lookup_internal(prefix, namespaces = _calculate_namespaces) # :nodoc:
+      prefix = (prefix == '') ? 'xmlns' : prefix.delete_prefix("xmlns:")
+      ns = namespaces[prefix]
+      ns = '' if ns.nil? and prefix == 'xmlns'
+      ns
+    end
+
+    def _calculate_namespaces(cache_hash = nil, attlist_mappings = nil) # :nodoc:
+      return cache_hash[self] if cache_hash && cache_hash.key?(self)
+
+      attlist_mappings ||= document&.doctype&._attlist_mappings || {}
+      inherited_namespaces = parent ? parent._calculate_namespaces(cache_hash, attlist_mappings) : {}
+      attr_namespaces = attributes._calculate_namespaces(attlist_mappings)
+      namespaces = attr_namespaces.any? ? inherited_namespaces.merge(attr_namespaces) : inherited_namespaces
+      cache_hash[self] = namespaces if cache_hash
+      namespaces
     end
 
+    private
+
     def __to_xpath_helper node
       rv = node.expanded_name.clone
       if node.parent
@@ -2297,7 +2306,7 @@ def each
     #   attrs.get_attribute('nosuch')        # => nil
     #
     def get_attribute( name )
-      fetch(name, nil) || attlist_attributes&.[](name)
+      fetch(name, nil) || attlist_attributes[name]
     end
 
     # :call-seq:
@@ -2368,11 +2377,7 @@ def prefixes
     #   d.root.attributes.namespaces # => {"xmlns"=>"foo", "x"=>"bar", "y"=>"twee"}
     #
     def namespaces
-      namespaces = {}
-      each_effective_attribute do |attribute|
-        namespaces[attribute.name] = attribute.value if attribute.namespace_declaration?
-      end
-      namespaces
+      _calculate_namespaces
     end
 
     # :call-seq:
@@ -2503,33 +2508,40 @@ def get_attribute_ns(namespace, name)
       end
     end
 
+    def _calculate_namespaces(attlist_mappings = nil) # :nodoc:
+      namespaces = {}
+      each_effective_attribute(attlist_mappings) do |attribute|
+        namespaces[attribute.name] = attribute.value if attribute.namespace_declaration?
+      end
+      namespaces
+    end
+
     private
 
     # Iterates over the attributes of the element and those in the DTD.
     # If the element has an attribute with the same name as one in the DTD,
     # the element's attribute takes precedence.
-    def each_effective_attribute
-      return to_enum(__method__) unless block_given?
-      attr = attlist_attributes
-      attr = attr ? attr.merge(self) : self
-      attr.each_value do |attribute|
+    def each_effective_attribute(attlist_mappings = nil)
+      return to_enum(__method__, attlist_mappings) unless block_given?
+      attributes = attlist_attributes(attlist_mappings).merge(self)
+      attributes.each_value do |attribute|
         yield attribute
       end
     end
 
     alias each_own_attribute each_value
     private :each_own_attribute
 
-    def attlist_attributes
-      doctype = @element.document&.doctype
-      if doctype
-        expn = @element.is_a?(Document) ? doctype.name : @element.expanded_name
-        doctype.attribute_declarations_of(expn).map do |key, value|
-          attr = Attribute.new(key, value)
-          attr.element = @element
-          [key, attr]
-        end.to_h
-      end
+    def attlist_attributes(attlist_mappings = nil)
+      attlist_mappings ||= @element.document&.doctype&._attlist_mappings
+      return {} unless attlist_mappings
+
+      expn = @element.is_a?(Document) ? @element.doctype&.name : @element.expanded_name
+      attlist_mappings[expn]&.map do |key, value|
+        attr = Attribute.new(key, value)
+        attr.element = @element
+        [key, attr]
+      end.to_h
     end
   end
 end

lib/rexml/xpath_parser.rb

@@ -63,6 +63,9 @@ def initialize(strict: false)
       @namespaces = nil
       @variables = {}
       @functions = FunctionsClass.new
+      @attlist_mappings = nil
+      @document = nil
+      @element_namespaces_cache = {}
       @nest = 0
       @strict = strict
     end
@@ -85,14 +88,8 @@ def parse path, node
         node = node.first
       end
 
-      document = node.document
-      if document
-        document.__send__(:enable_cache) do
-          match( path_stack, node )
-        end
-      else
-        match( path_stack, node )
-      end
+      @document = node.document
+      match(path_stack, node)
     end
 
     def get_first path, node
@@ -167,20 +164,45 @@ def strict?
       @strict
     end
 
-    # Returns a String namespace for a node, given a prefix
+    # Returns a String namespace for a prefix used in xpath
     # The rules are:
     #
     #  1. Use the supplied namespace mapping first.
-    #  2. If no mapping was supplied, use the context node to look up the namespace
-    def get_namespace( node, prefix )
+    #  2. If no mapping was supplied, use the context node to look up the namespace as a fallback.
+    def get_xpath_namespace(node, prefix)
       if @namespaces
         @namespaces[prefix] || ''
+      elsif node.node_type == :element
+        element_namespace_lookup(node, prefix)
       else
-        return node.namespace( prefix ) if node.node_type == :element
         ''
       end
     end
 
+    # Returns attribute's namespace URI while caching the
+    # intermediate result to speed up retrieval of namespaces
+    def attribute_namespace(attribute)
+      attribute.prefix == '' ? '' : element_namespace_lookup(attribute.element, attribute.prefix)
+    end
+
+    # Return element's namespace URI while caching the
+    # intermediate result to speed up retrieval of namespaces
+    def element_namespace(element)
+      element._namespace_internal(element_namespaces(element))
+    end
+
+    # Returns a hash of namespaces for the given element while caching the
+    # intermediate result to speed up retrieval of namespaces
+    def element_namespaces(element)
+      @attlist_mappings ||= @document&.doctype&._attlist_mappings || {}
+      element._calculate_namespaces(@element_namespaces_cache, @attlist_mappings)
+    end
+
+    # Returns namespace of the prefix in the context of the element,
+    # while caching the intermediate result to speed up retrieval of namespaces
+    def element_namespace_lookup(element, prefix)
+      element._namespace_lookup_internal(prefix, element_namespaces(element))
+    end
 
     # Expr takes a stack of path elements and a set of nodes (either a Parent
     # or an Array and returns an Array of matching nodes
@@ -641,20 +663,20 @@ def node_test(path_stack, any_type: :element)
               node.name == name
             elsif prefix.empty?
               if strict?
-                node.name == name and node.namespace == ""
+                node.name == name and element_namespace(node) == ""
               else
-                node.name == name and node.namespace == get_namespace(node, prefix)
+                node.name == name and element_namespace(node) == get_xpath_namespace(node, prefix)
               end
             else
-              node.name == name and node.namespace == get_namespace(node, prefix)
+              node.name == name and element_namespace(node) == get_xpath_namespace(node, prefix)
             end
           when :attribute
             if prefix.nil?
               node.name == name
             elsif prefix.empty?
-              node.name == name and node.namespace == ""
+              node.name == name and attribute_namespace(node) == ""
             else
-              node.name == name and node.namespace == get_namespace(node.element, prefix)
+              node.name == name and attribute_namespace(node) == get_xpath_namespace(node.element, prefix)
             end
           else
             false
@@ -665,11 +687,11 @@ def node_test(path_stack, any_type: :element)
         ->(node) do
           case node.node_type
           when :element
-            namespaces = @namespaces || node.namespaces
-            node.namespace == namespaces[prefix]
+            namespaces = @namespaces || element_namespaces(node)
+            element_namespace(node) == namespaces[prefix]
           when :attribute
-            namespaces = @namespaces || node.element.namespaces
-            node.namespace == namespaces[prefix]
+            namespaces = @namespaces || element_namespaces(node.element)
+            attribute_namespace(node) == namespaces[prefix]
           else
             false
           end

test/test_namespace.rb

@@ -1,7 +1,9 @@
 # frozen_string_literal: false
+require "core_assertions"
 
 module REXMLTests
   class TestNamespace < Test::Unit::TestCase
+    include Test::Unit::CoreAssertions
     include Helper::Fixture
     include REXML
 
@@ -34,5 +36,25 @@ def test_xml_namespace
       assert_equal("http://www.w3.org/XML/1998/namespace",
                    document.root.namespace("xml"))
     end
+
+    def test_deep_element_namespace_linear
+      omit('Recursion too deep on JRuby') if RUBY_ENGINE == "jruby"
+
+      max_depth = 3000
+      xml = <<~XML
+        <root xmlns="one">#{'<a>' * max_depth + '</a>' * max_depth}</root>
+      XML
+      doc = Document.new(xml)
+      prepare_element = ->(depth) do
+        node = doc.root
+        depth.times { node = node.first }
+        node || raise
+      end
+
+      assert_linear_performance([30, 100, 300, 1000, 3000], rehearsal: 10) do |depth|
+        elem = prepare_element.call(depth)
+        elem.namespace
+      end
+    end
   end
 end

test/xpath/test_base.rb

@@ -1312,16 +1312,6 @@ def test_namespaces_0
       assert_equal( 1,  XPath.match( d, "//x:*" ).size )
     end
 
-    def test_namespaces_cache
-      doc = Document.new("<a xmlns='1'><b/></a>")
-      assert_equal("<b/>", XPath.first(doc, "//b[namespace-uri()='1']").to_s)
-      assert_nil(XPath.first(doc, "//b[namespace-uri()='']"))
-
-      doc.root.delete_namespace
-      assert_nil(XPath.first(doc, "//b[namespace-uri()='1']"))
-      assert_equal("<b/>", XPath.first(doc, "//b[namespace-uri()='']").to_s)
-    end
-
     def test_ticket_71
       doc = Document.new(%Q{<root xmlns:ns1="xyz" xmlns:ns2="123"><element ns1:attrname="foo" ns2:attrname="bar"/></root>})
       el = doc.root.elements[1]