Skip to content

perf: stop flushing per token while writing the final parse tree (1.10x) - #720

Merged
ddehilster merged 1 commit into
masterfrom
perf/tree-dump-flush
Aug 25, 2026
Merged

perf: stop flushing per token while writing the final parse tree (1.10x)#720
ddehilster merged 1 commit into
masterfrom
perf/tree-dump-flush

Conversation

@ddehilster

Copy link
Copy Markdown
Member

What I was chasing

After 3.8.7, a profile of a compiled parse-en-us run still had ~13% of self time in ntdll!ZwWriteFile. My first guess was the write path itself — lite/var.cpp opens every NLP++ output file with std::ios::app, which on the MSVC CRT repositions to end-of-file before every write.

That guess was wrong, and worth recording. A standalone benchmark writing the same 6.1 MB through std::ofstream in 220k small << calls:

ios::app, default buffer (engine today) 0.034 s 170.5 MB/s
ios::app, 64 KB buffer 0.033 s 179.4 MB/s
in|out|ate, default buffer 0.031 s 192.2 MB/s
in|out|ate, 64 KB buffer 0.029 s 204.5 MB/s

Append mode costs 13%, buffer size 5%, and the whole 6 MB costs 34 ms. So neither append mode nor buffering explains ~1.3s — buffered stream writes were never the problem.

What it actually is

Going back to the stacks:

Parse::finExecute -> Parse::finalTree -> Tree<Pn>::Traverse
-> operator<< -> Iarg::genArg -> std::flush (11.5% inclusive)

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 costs a handful of WriteFile syscalls. It's the same defect as the Arun::out per-write flush fixed in 3.8.5, just in a different writer.

Result

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:

minp25median
flush per token7.69s7.73s8.12s
buffered7.00s7.00s7.16s

1.10x.

Correctness

All 18 analyzer output files byte-identical, final.tree included (3,085,335 bytes before and after).

Iarg::genArg is also used when writing generated .nlp rule files. That's a bulk writer too and it flushes on close, so the content is unaffected there as well — but flagging it since it's a second caller.

Version

Assumes #718 (3.8.8) lands first. If it doesn't, this should be 3.8.8.

🤖 Generated with Claude Code

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>
@ddehilster

Copy link
Copy Markdown
MemberAuthor

Rebased onto master now that #718 and #721 have landed, and resolved the version to 3.8.9 (master is at 3.8.8).

Only the version line needed resolving — this PR touches lite/iarg.cpp and #718 touched lite/irule.cpp/lite/ifile.cpp, so the code never overlapped.

This run is also the first real test of the vcpkg binary cache from #721: the entries were saved on that PR's run, so Install 3rd Party should drop from ~10 minutes to near-zero here.

@ddehilster
ddehilster merged commit 7c3dc8c into masterAug 25, 2026
7 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@ddehilster