PERF: Speed up the BBCode conversion hot paths#58
Merged
Conversation
This was referenced Jul 8, 2026
gschlager
force-pushed
the
perf-hot-paths
branch
2 times, most recently
from
f0a45d0 to
6711b7d
Compare
gschlager
changed the title
These are the instruments behind the perf work on the BBCode pipeline. bench/corpus_bench.rb runs a deterministic 200-post corpus (ASCII and multibyte prose, same structure) through isolating variants — the differences between variants localize cost: fresh minus shared is the per-call setup, parse_only minus scan_only is handler/AST construction, and ascii vs multi exposes character-index pathologies. Both scripts share a variant registry keyed by corpus, so further source formats plug in as single entries. bench/alloc_profile.rb reports objects/post via GC.stat and ranks the exact allocation sites by file:line via ObjectSpace. bench.rb loses its mixed and large_doc reports — pseudo-corpus inputs that the real corpus supersedes. The per-feature micro reports and the isolated escape_* reports (the escaper rule's measuring stick) stay.
Building HandlerRegistry.default (~20 handler objects plus closing strategies) and TagLibrary.default (a constant scan plus ~30 tag instantiations) on every bbcode_to_markdown call was about 40% of the default conversion path on the corpus bench (fresh 88.1 vs shared 52.6 µs/post; the setup_only variant isolates 26 µs of pure construction). Handlers, tags, and closing strategies hold no per-parse state (that all lives in ParserState), so the no-customization paths in Parser and Renderer now fall back to shared, deep-frozen instances built once per process. Customization paths (Parser.new with a block, explicit handlers:/tag_library:, discourse_renderer) still get fresh instances. freeze is overridden on both classes to also freeze the internal collections, so register on a shared instance raises FrozenError instead of silently mutating state visible to every parser. In HandlerRegistry#freeze the raise always fires on @handlers before the other two collections are reachable, which makes drops of their .freeze lines unobservable — that subject goes on the mutant ignore list with a comment. Corpus bench: fresh/ascii 88.1 → 45.5 µs/post, fresh/multi 126.6 → 90.0 µs/post.
CRuby has no character-index cache: on any string containing a multibyte character, str[pos] and str.index(pattern, pos) walk bytes from the start of the string — O(pos) per call and superlinear per document. That made scanning multibyte posts ~4.8x more expensive than ASCII ones. On top of that, current_char allocated a one-character String for every probe (~54 per post). The scanner now works entirely in byte offsets (byteindex, byteslice, getbyte) and classifies bytes with integer range checks instead of regex probes over one-char strings. The invariant that every offset sits on a character boundary is held structurally: byteindex jumps land on ASCII matches, manual advances step over ASCII bytes. Token#pos is now a byte offset; nothing outside the scanner consumes it. Corpus bench: scan_only 37.1 → 4.0 µs/post multibyte (9.2x) and 7.8 → 3.6 ASCII; parse_only 62.3 → 25.9 µs/post multibyte. New specs pin every byte-class range endpoint plus its adjacent bytes (the boundary mutations regexes never had), and put multibyte text in front of each construct so a byte/char-index mixup can't hide. Scanner#scan_while is gone (replaced by scan_attr_name, whose loop-bound mutations are killable), so its ignore entry goes too.
Five changes, each named by the allocation profiler (bench/alloc_profile.rb); together they take the default path from 622 to 199 objects/post on the corpus bench: - HandlerRegistry#[] no longer allocates a downcased copy per token lookup. The scanner already emits downcased tags, so a fetch with a normalizing fallback block hits directly in the common case. - AST::Text shares frozen input strings instead of duping every text node (copy-on-write; merge dups before its first append). The parser always passes frozen token text, so this drops an allocation and a memcpy per text node. Text built from a mutable string is still copied, so mutations can't leak in either direction. - Parser#normalize_line_endings skips the gsub copy when the input has no CR/LS/PS to normalize (one full-document String per post). - RenderingInterface#wrap_inline skips the capture-group sub (MatchData plus four capture Strings per inline tag) when the content has no leading/trailing whitespace; the whitespace-preserving sub moved into a private apply_markers helper. - RawHandler resolves language: support once at construction instead of reflecting over the element class per [code] element. Handlers are long-lived; the answer can't change between elements. The fetch fast path, the normalize guard, and the apply_markers fast path are output-equivalent to their slow paths, so their surviving mutations only differ in allocations — each subject is on the mutant ignore list with a comment. The sub's regex semantics (/m, edge quantifiers) and the copy-on-write contract are pinned by new specs.
The scanner guidance in AGENTS.md and docs/performance.md still told people to use character-index access (@input[@pos]) — the exact pattern the byte-offset rewrite removed — and docs/performance.md still claimed the RenderContext parent-lookup cache that was dropped during the mutation-coverage work. Both now describe the byte-domain rules (boundary invariant, nil-check byteindex, integer byte classes) and point at bench/corpus_bench.rb and bench/alloc_profile.rb as the measure-first instruments. The escaper benchmarking note pointed at /tmp/bench_escaper.rb, a file that no longer exists; it now names the isolated escape reports in bench/bench.rb.
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.
I want Markbridge to hold up on large conversions (think millions of posts), so I profiled the BBCode pipeline and fixed the biggest cost centers. The instruments are part of the PR:
bench/corpus_bench.rb(~1 KB forum-post corpus, ASCII + multibyte, isolating variants) andbench/alloc_profile.rb(allocations ranked by file:line).What changed:
HandlerRegistry.defaultandTagLibrary.defaulton every call — about 40% of the total. It now uses shared, deep-frozen instances built once per process; mutating one raisesFrozenError, customization paths still get fresh ones.byteindex/byteslice/getbytewith integer byte classes). Character indices are O(pos) on multibyte strings in CRuby, which made multibyte posts ~5x slower to scan.AST::Text, gsub/sub fast paths in line-ending normalization andwrap_inline,RawHandlerreflection memoized.bbcode_to_markdown, ASCIIbbcode_to_markdown, multibyteMutation coverage stays at 100%: the byte-range predicates get boundary specs probing every range endpoint and adjacent byte, and new multibyte regression specs place multibyte text before each construct. Four fast-path subjects went on the
mutant.ymlignore list with comments (slow path produces identical output, only more allocations).Also updated AGENTS.md and docs/performance.md — both still described the old character-index scanner. Deliberately not touched:
MarkdownEscaperinternals beyond docs, andRenderContext(that's #59).