perf: FxHash node-cache + direct attr-value reads (output-identical, ~28-30% on node-heavy docs)#205
Open
dginev wants to merge 2 commits into
Open
perf: FxHash node-cache + direct attr-value reads (output-identical, ~28-30% on node-heavy docs)#205dginev wants to merge 2 commits into
dginev wants to merge 2 commits into
Conversation
The internal node bookkeeping map (probed on every Node::wrap/lookup) used the std SipHash RandomState. Keys are allocator-chosen xmlNodePtr addresses (not adversarial), so HashDoS resistance is irrelevant and SipHash is wasted work. A dependency-free FxHash-style multiply-rotate hasher on the pointer key cuts ~28-30% of wall on math/node-heavy documents (measured downstream in latexml-oxide: 1510.03361 19.6s->14.1s, tikz-cd 22.4s->15.7s, both phases). Zero new dependencies (std::hash only); the map is never iterated so losing per-run randomization is a no-op; output-identical. Includes a determinism + no-collision regression test. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
get_properties / get_properties_ns (and the get_attributes aliases) looped over the attribute list but then re-resolved each value by name via get_property -> xmlGetProp, which re-scans the whole attribute list (xmlStrEqual per entry) and allocates a fresh CString for the name on every attribute. That is quadratic in the attribute count and pure overhead, since the loop already holds the attribute node pointer. Read the value straight from that node with xmlNodeGetContent (the same call get_content already uses), via a small attr_node_value helper. Returned map is unchanged; libxml's own attribute-accessor tests pass. Also documents the prior FxHash node-cache change under [Unreleased].
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
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.
Two dependency-free, output-identical performance improvements to the hot paths
exercised by node/attribute-heavy XML processing (measured downstream in
latexml-oxide, a Rust port of LaTeXML).
Both are based on the latest
main.1.
perf(node-cache): FxHash-style hasher for thexmlNodePtr -> NodemapThe internal node-bookkeeping map is probed on every
Node::wrap/lookup and usedthe std
SipHashRandomState. The keys are allocator-chosenxmlNodePtraddresses (not adversarial input), so HashDoS resistance is irrelevant and SipHash
is pure wasted work. Swapping in a dependency-free FxHash-style multiply-rotate
hasher on the pointer key cuts ~28-30% of wall on math/node-heavy documents
(latexml-oxide:
1510.0336119.6s→14.1s, a tikz-cd paper 22.4s→15.7s, across bothconversion phases).
std::hashonly).2.
perf(attrs):get_propertiesreads values directly from the attr nodeget_properties/get_properties_ns(and theget_attributesaliases) loopedover the attribute list but then re-resolved each value by name via
get_property→xmlGetProp, which re-scans the whole attribute list(
xmlStrEqualper entry) and allocates a freshCStringfor the name on everyattribute — quadratic in the attribute count, and pure overhead since the loop
already holds the attribute node pointer.
This reads the value straight from that node with
xmlNodeGetContent(the samecall
get_contentalready uses), via a smallattr_node_valuehelper. Thereturned map is unchanged; libxml's own attribute-accessor tests pass.
Validation
cargo test --release— all test binaries pass (0 failed), including the newnode-cache determinism test.