Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions docs/performance.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,37 @@ Always prefer SAX for validation. Only use DOM when applying remediations.
* *Class-level cache*: Use for data shared across all instances
* *Thread safety*: Always use Mutex for class-level shared state

=== Cache Key Stability for XML Node Identity

When caching node-related data across traversals, ensure cache keys use stable node identity.

*Problem:* Moxml wraps underlying parser nodes (Nokogiri/Oga) in new wrapper objects on each traversal. Using `wrapper.object_id` as a cache key causes cache misses since the wrapper object changes between traverses.

*Solution:* Use `node.native.object_id` as the cache key since the underlying parser node identity is stable:

[source,ruby]
----
# Wrong - wrapper object_id changes across traverses
@cache[node.object_id] = value

# Correct - native node identity is stable
@cache[node.native.object_id] = value
----

*Example:* `DocumentAnalyzer` uses this pattern for node ID caching:

[source,ruby]
----
def get_node_id(node)
native_id = node.native.object_id
@cache[native_id] ||= compute_and_cache_path(node)
end
----

*When to use:* When caching data associated with XML nodes that may be accessed via different wrapper instances.

*Why it works:* The underlying Nokogiri/Oga/other parser nodes maintain stable Ruby object identity across wrapper re-creation.

== Future Optimization Opportunities

Areas that may benefit from future optimization:
Expand Down
13 changes: 9 additions & 4 deletions lib/svg_conform/document_analyzer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,10 @@ def initialize(document)
def get_node_id(node)
return nil unless node.respond_to?(:name) && node.name

# Return cached ID or compute on-demand
@cache[node.object_id] ||= compute_and_cache_path(node)
# Use native.object_id as cache key since wrapper object_id changes
# across traverses but underlying Nokogiri node identity is stable
native_id = node.native.object_id
@cache[native_id] ||= compute_and_cache_path(node)
end

private
Expand Down Expand Up @@ -63,7 +65,7 @@ def populate_cache
end.compact
path_parts << "#{node.name}[#{position}]"

@cache[node.object_id] = "/#{path_parts.join('/')}"
@cache[node.native.object_id] = "/#{path_parts.join('/')}"
end
end

Expand Down Expand Up @@ -104,12 +106,15 @@ def calculate_position_fast(node)
return 1 unless parent.respond_to?(:children)

# Count this node's position among siblings with same name
# Use native.object_id comparison since wrapper objects differ
# across traverses but underlying Nokogiri nodes are stable
position = 0
native_id = node.native.object_id
parent.children.each do |child|
next unless child.respond_to?(:name) && child.name == node.name

position += 1
break if child.equal?(node)
break if child.native.object_id == native_id
end

position
Expand Down
Loading