Uh oh!
There was an error while loading. Please reload this page.
perf(codegen): emit one function per rule instead of one giant switch - #718
Merged
Conversation
ddehilster
changed the base branch from
perf/compiled-runtime-output-and-matching
to
masterAugust 25, 2026 14:07
Irule::genRule emitted every rule in a pass as a `case` of a single `switch (ruleno)` inside one matchRule<id>() function. MSVC lays out the union of all the cases' locals in one frame, so the frame grew with the pass. In parse-en-us: matchRule75 89,404 bytes of stack matchRule48 46,472 bytes Every rule attempt paid _chkstk touching ~22 stack pages before doing any matching. Profiling a compiled run put _chkstk at 5.9% of self time. This is not a case for disabling stack probes: building with /Gs1048576 crashes with an access violation, which is what a frame that large means. Emit one function per rule, so each frame holds only what that rule needs, and leave matchRule<id>() as a dispatcher. Safe because the generated rule bodies use no goto/continue, never touch `done` (only the default arm does), and never use pcoll -- verified across all 139 generated passes of parse-en-us. Dispatch goes through a table of function pointers rather than a switch calling each function directly. Each per-rule function has exactly one call site, so the compiler inlines them all straight back into the dispatcher and rebuilds the frame we just split up -- measured: zero per-rule functions survived and matchRule75 still asked for 89,404 bytes. The per-rule functions are also marked noinline. MSVC does not need that once dispatch is indirect, but it keeps the frame-size guarantee from resting on one compiler's inlining heuristics, which matters because analyzers are also compiled with gcc and clang. After: no generated dispatcher needs a stack probe at all, _chkstk disappears from the profile, and matchRule75 self time goes 6.4% -> 0.6%. Interleaved A/B, 8 runs per arm, swapping only run.dll: one big switch min 7.63s p25 7.73s median 8.03s one fn per rule min 7.31s p25 7.40s median 7.68s 1.04x - 1.05x Modest, because only the probe overhead goes away -- the rule bodies still do the same work, just in their own frames. An earlier run suggested 13%, but that was measurement noise from background load; interleaving the two DLLs on a quiet machine gives the number above. All 18 analyzer output files are byte-identical before and after. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ddehilsterforce-pushed
the
perf/one-function-per-rule
branch
from
August 25, 2026 15:39
a5796ca to
ec03762Compareddehilster
commented
Aug 25, 2026
MemberAuthor
Rebased onto master and bumped 3.8.6 → 3.8.8. v3.8.7 has been tagged and released, and it contains #717 and #719 but not this PR — so the 3.8.6 bump this branch carried would have set the version backward. Git caught it as a conflict in No change to the code itself; only the version line moved. |
This was referenced Aug 25, 2026
Uh oh!
There was an error while loading. Please reload this page.
ddehilster added a commit
that referenced
this pull request
Aug 25, 2026
Profiling a compiled parse-en-us run after 3.8.7 left ~13% of self time in ntdll!ZwWriteFile. That is not write volume: a plain ofstream writes the same 6.1 MB in 0.034s at 170 MB/s. The stacks pointed at Parse::finExecute -> Parse::finalTree -> Tree<Pn>::Traverse -> operator<< -> Iarg::genArg -> std::flush Parse::finalTree writes a 3 MB final.tree, and Iarg::genArg/genArgs flushed after nearly every fragment they emitted -- 22 std::flush calls on that path, so each node's attribute list cost a handful of WriteFile syscalls. Same defect as the Arun::out per-write flush fixed in 3.8.5, in a different writer. Removed; the stream still flushes when it closes, so the file on disk is unchanged. Interleaved A/B, 8 runs per arm, swapping only nlp.exe: flush per token min 7.69s p25 7.73s median 8.12s buffered min 7.00s p25 7.00s median 7.16s 1.10x All 18 analyzer output files byte-identical, final.tree included (3,085,335 bytes before and after). Version note: assumes #718 (3.8.8) lands first. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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 freeto 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.
Stacked on #717 — retarget to
masteronce that merges.The problem
Irule::genRuleemitted every rule in a pass as acaseof a singleswitch (ruleno)inside onematchRule<id>(). MSVC lays out the union of all the cases' locals in one frame, so the frame grew with the pass. In parse-en-us:Every rule attempt paid
_chkstktouching ~22 stack pages before doing any matching — 5.9% of self time in a profile of a compiled run.This is not a case for turning off stack probes. Building with
/Gs1048576crashes with an access violation, which is exactly what a frame that large means.The change
One function per rule, with
matchRule<id>()reduced to a dispatcher. Safe because generated rule bodies use nogoto/continue, never touchdone(only the default arm does), and never usepcoll— verified across all 139 generated passes of parse-en-us.Dispatch goes through a table of function pointers, not a switch calling each function directly. That detail matters: each per-rule function has exactly one call site, so the compiler inlines them all straight back into the dispatcher and rebuilds the frame. Measured — with a plain switch, zero per-rule functions survived in
run.dllandmatchRule75still asked for 89,404 bytes.The per-rule functions are also marked
noinline. MSVC doesn't need it once dispatch is indirect (verified), but it keeps the frame-size guarantee from resting on one compiler's inlining heuristics, and analyzers are compiled with gcc and clang too.Results
No generated dispatcher needs a stack probe at all.
_chkstkdisappears from the profile;matchRule75self time goes 6.4% → 0.6%.Interleaved A/B, 8 runs per arm, swapping only
run.dll:1.04x – 1.05x. Modest, because only the probe overhead goes away — the rule bodies still do the same work, just in their own frames.
An earlier measurement suggested 13%; that was noise from background load. Interleaving the two DLLs on a quiet machine gives the number above. Flagging it because the honest figure is the smaller one.
Correctness
All 18 analyzer output files byte-identical before and after.
🤖 Generated with Claude Code