PERF: Cut the HTML and TextFormatter tree-walk overhead#61
Merged
Conversation
ba1cd32 to
b140416
Compare
129d584 to
41004c3
Compare
b140416 to
c0afe79
Compare
41004c3 to
7e6e142
Compare
c0afe79 to
bc3fc4e
Compare
7e6e142 to
cfb1d9f
Compare
bc3fc4e to
c069882
Compare
cfb1d9f to
a312398
Compare
c069882 to
ff0eb41
Compare
a312398 to
c28c90d
Compare
ff0eb41 to
53166fb
Compare
c28c90d to
40378af
Compare
The s9e/TextFormatter corpus mirrors the other formats (ASCII + multibyte, mixed constructs, <s>/<e> markup preservation). New html_walk/html_nokogiri and tf_walk/tf_nokogiri variants separate nokogiri's C parse from the Ruby tree walk by pre-parsing the corpus and feeding the parsers Nokogiri nodes. Baseline these produced: the HTML walk costs 36.7 µs/post against 21.7 µs for the nokogiri parse itself — the Ruby side is the bigger half. The allocation profile blames node.text.gsub copies per text node, a node.ancestors walk per text node, and rstrip+pop+re-add churn per element. TextFormatter is much leaner (7.6 µs walk); its only notable waste is per-call registry construction and two array literals allocated per element.
The walk cost more than nokogiri's actual parse (36.7 vs 21.7 µs/post on the corpus). Four changes, profiler-ranked: - Whitespace preservation is now tracked with a counter during descent instead of walking node.ancestors for every text node. The counter is seeded once per parse from the parse root's real ancestry, so pre-parsed subtrees inside a <pre> keep their exact semantics (new specs pin that, plus custom-preserving-tag nesting — the built-in preserving tags use RawHandler and never walk children). - The default HandlerRegistry is built once per process (shared, deep-frozen — same pattern as the other parsers). All three internal collections are publicly reachable, so every freeze line is pinned by a raising spec. - process_text_node only pays the whitespace-collapsing gsub when the text contains something collapsible; single-space prose passes through without the copy. - trim_trailing_whitespace exits early when the last text node has no strippable tail, instead of rstrip+pop+re-add on every element. The two gsub/rstrip gates are output-equivalent fast paths and join the mutant ignore list with the registry fetch lookups; everything else is at 100% with killing specs. Corpus bench: html_walk 36.7 → 16.9 µs/post (allocations 285 → 143), full HTML conversion 87.9 → 60.8 ASCII and 117.3 → 81.3 multibyte. Output verified unchanged against the old implementation over all 400 corpus posts plus Unicode edge cases (zero diff, unknown_tags included).
Small wins — the TextFormatter walk was already lean: - The default HandlerRegistry is built once per process (shared, deep-frozen), instead of a full construction per parse call. - The %w[s e] / %w[t r] literals allocated two Arrays per element node; they're frozen constants now. - Registry lookups fetch directly (s9e stores tag names uppercase, so the fetch hits without the upcase copy); the fallback keeps case-insensitive lookups working. Corpus bench: tf_fresh 35.4 → 29.8 µs/post ASCII, tf_parse allocations drop by the per-parse registry share. Output verified unchanged (zero diff over the corpus + Unicode edge cases).
53166fb to
d2cda94
Compare
40378af to
7eac804
Compare
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.
Follow-up to #58/#59/#60, covering the two remaining parsers. New bench variants separate nokogiri's C parse from the Ruby tree walk by feeding the parsers pre-parsed nodes — and for HTML, the walk turned out to cost more than the parse (36.7 vs 21.7 µs/post).
HTML, profiler-ranked:
node.ancestorsfor every text node. The counter is seeded once per parse from the parse root's real ancestry, so pre-parsed subtrees inside a<pre>keep their exact semantics.HandlerRegistryis built once per process (shared, deep-frozen, same pattern as the other parsers).process_text_nodeonly pays the whitespace-collapsing gsub when the text contains something collapsible, andtrim_trailing_whitespaceexits early when there is nothing to strip.TextFormatter was already lean; it gets the shared registry, frozen constants for the
%w[s e]/%w[t r]arrays it allocated per element, and the direct-fetch lookup.Output is unchanged: zero diff of rendered Markdown (including unknown_tags) over all 400 HTML and 400 TextFormatter corpus posts plus Unicode edge cases, old vs new.
Mutation coverage stays at 100% on both namespaces. The gsub/rstrip gates are output-equivalent fast paths and join the existing ignore entries; the preserve-depth counter is pinned by new specs, including nesting through a custom preserving tag (the built-in ones use RawHandler and never walk children).
After this, HTML conversion is roughly half nokogiri — there is not much left on the Ruby side of these two parsers.