Uh oh!
There was an error while loading. Please reload this page.
perf(build): unity + /MP for analyzer compiles (16x), guard three kbm headers - #719
Merged
Conversation
…eaders Compiling a generated analyzer is dominated by re-parsing the same headers. parse-en-us emits 520 kb/*.cpp (57 MB) plus 139 run/*.cpp, and an 80 KB generated source expands to ~4.5 MB of engine headers -- about 45% of each translation unit's compile time is that header block, paid 520 times. Two fixes, both measured on parse-en-us: 1. UNITY_BUILD batches the generated sources so the header block is parsed once per batch instead of once per file. 0.71 s/file -> 0.039 s/file at batch 65. 2. /MP. `cmake --build --parallel N` with the Visual Studio generator becomes msbuild /m:N, which parallelises PROJECTS -- there is one project, so without /MP the sources compile serially no matter what --parallel says. Measured on 20 files with --parallel 4: 15.08s without /MP, 4.29s with. End to end, a clean 100-file CMake build went 61.79s -> 3.87s (16x) with identical exported symbol sets, and a full parse-en-us kb+run build (528 files, 57 MB) takes ~30s where it used to take 10-15 minutes. Batch size is derived from the core count (~2 batches per core, clamped to [8,64]) so small analyzers still spread across cores and large ones do not build one enormous TU. -DNLP_UNITY_BUILD=OFF falls back to one TU per file. UNITY_BUILD needs the include guards: kbm/sym_s.h, kbm/con_s.h and kbm/ptr_s.h had none, so a second inclusion inside one TU fails with C2011 'sym': 'struct' type redefinition. These headers ship in nlpengine-compile-libs, so the cloud compile service needs a release carrying this commit before it can turn unity on at its end. Version note: assumes #718 (3.8.6) lands first. If it does not, this should be 3.8.6. 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.
Compiling a generated analyzer is dominated by re-parsing the same headers. parse-en-us emits 520
kb/*.cpp(57 MB) plus 139run/*.cpp, and an 80 KB generated source expands to 4.5 MB of engine headers — roughly 45% of each TU's compile time is that header block, paid 520 times.Two fixes
1.
UNITY_BUILDbatches the generated sources so the header block is parsed once per batch instead of once per file.2.
/MP. This one is a plain bug.cmake --build --parallel Nwith the Visual Studio generator becomesmsbuild /m:N, which parallelises projects — there is one project, so without/MPthe sources compile serially regardless of what--parallelsays. Measured on 20 files with--parallel 4on a 22-core box:Results
Clean 100-file CMake build, current config vs proposed: 61.79s → 3.87s (16x), with identical exported symbol sets (115 in both, diffed).
A full parse-en-us
kb+runbuild (528 files, 57 MB) now takes ~30s where it used to take 10–15 minutes — the figure thecompile-analyzer.ymlcomment refers to when it says "long builds (10+ min for large analyzers like parse-en-us) look hung".Batch size is derived from the core count (~2 batches per core, clamped to
[8,64]) so small analyzers still spread across cores and large ones don't build one enormous TU.-DNLP_UNITY_BUILD=OFFfalls back to one TU per file.Why the header guards are in here
UNITY_BUILDdoes not compile at all without them. kbm/sym_s.h, kbm/con_s.h and kbm/ptr_s.h had no include guards, so a second inclusion inside one TU fails withC2011: 'sym': 'struct' type redefinition.These headers ship inside
nlpengine-compile-libs, so the cloud compile service needs a release carrying this commit before it can turn unity on at its end. Companion PRs fornlp-compile-serviceandvscode-nlpfollow — all three generate the same CMakeLists and have to move together.Version
Assumes #718 (3.8.6) lands first. If it doesn't, this should be 3.8.6.
🤖 Generated with Claude Code