Conversation
…caching TODOs 1-5 from the moxml performance plan: 1. Lazy parse (TODO 1): Replace eager DocumentBuilder with Document.new(native, ctx). Nodes are wrapped on access, not during parse. ~36,640 allocations eliminated per 200-element doc. 2. Cache Node#children (TODO 2): Memoize children NodeSet with invalidation on mutation (add_child, remove, replace, siblings). ~4,000 redundant NodeSet allocations eliminated. 3. Cache Element#attributes/namespaces (TODO 3): Memoize with invalidation on attribute/namespace mutation. ~3,000 wrapper allocations eliminated. 4. Cache wrapped nodes in NodeSet (TODO 4): Per-index wrapped cache eliminates redundant Node.wrap calls during iteration. ~1,000 wrapper allocations eliminated. 5. Remove unnecessary allocations (TODO 5): Remove .dup from visit_children, set parent refs in Ox adapter children method, compact adapter test XML to avoid whitespace text node issues. Parent cache invalidation: Store parent_node on child wrappers via NodeSet, propagate invalidation upward on remove/replace/sibling ops. All adapters (Nokogiri, Ox, Oga, REXML) pass the shared contract tests. 73 performance specs pass under RUN_PERFORMANCE=1.
Add CI-gated allocation benchmarks covering Nokogiri, Ox, HeadedOx, and OGA adapters. These guards run in every CI build (no :performance tag) and enforce per-adapter allocation budgets for: - Parse allocations (100/50 element documents) - Cache hit verification (children, attributes, iteration) - Round-trip allocations (parse → serialize → parse) - Scalability (linear growth, not quadratic) - Cache invalidation (mutation breaks cache) - NodeSet wrap cache (identity across accesses) Measured baselines (allocations per 100-element parse): Nokogiri: 299 | Ox: 1003 | OGA: 8732 | HeadedOx: 176,472 HeadedOx thresholds are intentionally generous because it still uses DocumentBuilder (eager parse). Tighten after lazy parse migration. Also: - Add AllocationHelper support module with per-adapter thresholds - Add StackProf diagnostic on guard failure (allocation hotspots) - Extend lazy_parse, node_cache, node_set_cache specs to all adapters - Remove :performance tag from correctness tests (now run in CI) - Add stackprof to Gemfile
Replace DocumentBuilder (eager tree construction) with Document.new (lazy wrapping) in HeadedOx adapter, matching the Ox adapter approach. The XPath engine already wraps nodes on-demand via Moxml::Node.wrap, so the eager DocumentBuilder was pure overhead. Impact on 100-element parse: Before: 176472 allocations (3.93x scalability - nearly quadratic) After: 1001 allocations (2.0x scalability - linear) Also tighten HeadedOx allocation guard thresholds from the bloated DocumentBuilder budgets to match Ox-level budgets.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
DocumentBuilderwithDocument.new(native, ctx)— nodes are wrapped on access instead of recursively at parse timechildren,attributes, andnamespaceswith proper invalidation on structural mutations (add/remove/replace child)@wrappedarray eliminates redundantNode.wrapcalls when iterating children multiple times@parent_nodereferences propagated on remove/replace/sibling operations, enabling targeted cache invalidationArchitecture
The design follows the open/closed principle —
NodeandElementgained cache slots with invalidation hooks, while adapters remain thin wrappers. TheNodeSetwrap cache is transparent to consumers:each,[], and enumeration all benefit without API changes.Allocation reduction
For a typical XML document with ~500 elements, this eliminates:
Node.wrapcalls at parse time (lazy)Gating
New benchmark specs in
spec/moxml/allocation_benchmark_spec.rbverify allocation counts stay within bounds. Lazy parse, node cache, and NodeSet cache are all covered by dedicated specs.Test plan
bundle exec rspec— 193 adapter tests pass (0 failures)bundle exec rake spec— all performance specs pass (73 tests)lazy_parse_spec.rb,node_cache_spec.rb,node_set_cache_spec.rb,allocation_benchmark_spec.rb