Repository navigation
Build debug code frames only when debug output is enabled (faster text-domain stamping) - #51086
Merged
Merged
Conversation
`buildCodeFrameError()` renders an excerpt of the surrounding source, so its cost scales with the size of the file. Passing it straight to `debug()` reads as free when debug output is off, but JavaScript evaluates call arguments eagerly, so the frame was built for every matched call either way. That was cheap while this plugin ran over hand-written source with domains already present. Since #50762 it also runs over the esbuild bundles that `stamp-textdomains` post-processes, where the domain is absent by construction — so the missing-domain branch, the one that builds a frame, became the hot path on multi-megabyte files. Measured over premium-analytics' 342 built bundles: 211.8s -> 6.2s (34x), with byte-identical output for every file. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VDM1RKuvDmbfyMZspnghaa
Contributor
|
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
Contributor
|
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖 Follow this PR Review Process:
If you have questions about anything, reach out in #jetpack-developers for guidance! |
Code Coverage SummaryCoverage changed in 1 file.
|
louwie17
marked this pull request as ready for review
August 6, 2026 08:40
manzoorwanijk
previously approved these changes
Aug 6, 2026
manzoorwanijk
left a comment
Member
There was a problem hiding this comment.
Thank you. This looks good
Fixes the two jsdoc/require-hyphen-before-param-description warnings that failed the lint check. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VDM1RKuvDmbfyMZspnghaa
anomiex
added a commit
that referenced
this pull request
Aug 18, 2026
PR #50762 caused a significant rise in build times, which was fixed by #51086. Let's turn the build times back down after the fix. This effectively reverts #51029. As for #51059, the progression is 25 before → 40 during the rise → 30 now, to leave some leeway since full builds are up to ~18 minutes now.
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.
Proposed changes
Stamping translation text domains onto built JavaScript is the single most expensive step in a production build of
packages/premium-analytics— more expensive than all of esbuild put together. In the unmodified CI control it takes 11m54s of a 16m29s build; with this change, 2m44s, taking the whole package build to 8m29s.(Development builds are barely affected — see "What you will not see" below before measuring.)
In plain terms
When something looks unusual during text-domain stamping, the tool wants to log a helpful message showing the offending line, like a compiler error:
To produce that little excerpt, Babel has to scan the entire file to work out where line 12 is and what surrounds it.
The code asked for the excerpt like this:
debug(...)does nothing unless you have debug logging switched on. But JavaScript always evaluates the arguments before calling a function — so the excerpt was built every single time, then handed to a function that immediately discarded it.An analogy: printing and binding a 300-page report, walking it to the shredder, and repeating that 249 times — because the person you meant to give it to isn't in the building.
For one real bundle:
The fix checks whether anyone is listening first:
Debug output is unchanged when you switch it on (
DEBUG='@automattic/babel-plugin-replace-textdomain', orDEBUG='*'). The built files are byte-for-byte identical either way — this is purely wasted work being removed.Why it got bad now, and why it wasn't a problem before
This eager call has been in the plugin since #22077 (Dec 2021), where it was genuinely harmless: the plugin ran over hand-written source files, which are small and whose translation calls already have a text domain — so the expensive branch almost never ran.
#50762 added the
stamp-textdomainsstep, which runs this same plugin over the bundles@wordpress/build(esbuild) emits. Two things changed at once:@wordpress/buildstrips the text domain from gettext calls, which is the entire reasonstamp-textdomainsexists. "Domain argument is missing" is no longer an edge case; it is every call.So this is not a regression in #50762. It is a long-dormant inefficiency that #50762's new (and correct) usage turned into the dominant cost of a production build.
Measurements
Two end-to-end measurements of the real
stamp-textdomainsstep, both onpackages/premium-analyticsproduction builds. They differ, and the gap is informative, so both are given.Author's local measurement, timing the step on its own (342 bundles, nothing else running; results are machine-specific):
Independent local reproduction on the exact PR commit (350 stamped files, same unstamped production output for both implementations):
The independent run also confirmed that all 562 generated files were byte-for-byte identical between implementations. The wide difference between local ratios shows that the absolute local speedup depends heavily on the machine and workload.
In CI, in situ — the same step inside
Build all projects, where other projects are building concurrently and competing for the runner's cores:stamp-textdomainssteppackages/premium-analyticsbuildRuns: unmodified control · after
The CI ratio is the one worth planning against: a build agent is busy with other projects, so the saved CPU partly goes to whatever else is running rather than straight to wall time. Take ~4x on the step and ~8 minutes off this package as the observed CI figure.
Note the job totals on those two runs are not comparable — the build matrix differs by which projects each PR touches (140 projects vs 122). The per-project comparison is more useful, but is not perfectly controlled either: the control stamped 342 files while the later run stamped 350. Treat the CI figures as strong directional evidence rather than a laboratory benchmark.
Isolating the transform
On the author's machine, the Babel transform alone across all 342 built bundles, current plugin vs patched, with byte-identical output for every file, measured 211.8s → 6.2s (34x). An independent check of the current 2.48 MB dashboard bundle measured 1.52s → a median 0.53s (2.9x), while instrumenting
buildCodeFrameError()calls from 234 → 0. These results confirm the mechanism while also showing that the transform ratio is machine-specific.Once the transform stops dominating, the rest of
stamp-textdomainsbecomes the cost — writing files, re-hashing each sibling.asset.php, regenerating source maps, and the separate manifest pass. That residual is the obvious next lever (parallelising the file loop) if this step needs to get faster still. Out of scope here.Worst individual bundles, transform only:
routes/post-detail/content.jsroutes/dashboard/content.jsmodules/externals/index.jsTwo controls confirm the diagnosis rather than merely correlating with it:
routes/reports/content.jsimproves only 2.4x, because its translation calls already carry a domain, so the expensive branch rarely fires — exactly what the explanation predicts.What you will not see
A development build barely changes.
jetpack build packages/premium-analytics --depsruns the development build, which does not emit the ~171 minified bundles or runstrip-unminified-prod, so there is far less to stamp — @manzoorwanijk measured 95s → 81s that way. All the figures above are production builds (build-production), which is what CI runs and what ships. If you are checking this change by eye, use the production commands in the testing instructions below, or you will conclude it does almost nothing.The same plugin runs for every package that stamps text domains —
forms,activity-log,newsletter,publicize,videopress,seo,scan,jetpack-mu-wpcomand others — so the saving should be broad, but I have not measured those individually.Does this pull request change what data or activity we track or use?
No.
Testing instructions
1. Unit tests
All 43 tests and 51 existing snapshots pass. The snapshots being unchanged is itself the evidence that debug output and transform output are unaffected.
tests/code-frame.test.jscovers:To confirm the tests are not vacuous, revert the
if ( debug.enabled )guard insrc/index.jsand re-run: 4 of the 6 new tests fail.2. Timing a real build
Use a production build. A development build will show almost no difference, for the reason given above.
The package's normal
buildscript chains four steps, and the one this PR changes is the third:So run the first two as two separate commands.
build:wp-buildonly bundles — it does not stamp, and on its own will print no stamping output at all. That is deliberate: it leaves the bundles unstamped so the stamp step can then be timed by itself, instead of being buried inside one combined number.Run this command on both trunk and this branch:
time jetpack build packages/premium-analytics --deps --productionTest the textdomains stamp specifically:
Step 2 should end with a line like:
Run both steps on
trunkand again on this branch, comparing only the step 2 timing. The exact local ratio is machine-dependent: the author's measurement was 221.1s → 10.6s, while an independent reproduction was 21.38s → 11.29s. Inside the measured CI job the improvement was around 4x.Re-run step 1 before each step 2: stamping is idempotent, so timing it a second time on already-stamped output measures the cheap no-op path and will look fast on both branches.
For the in-situ figure, compare the
stamp-textdomainsline in the CIBuild all projectslog between the unmodified control and this branch; that is where 714s → 164s above comes from.3. Confirming debug output still works
DEBUG='@automattic/babel-plugin-replace-textdomain' pnpm run build:stamp-textdomainsThe "Domain argument (index 2) is missing" messages, with code frames, should appear exactly as before. Note that this deliberately restores the slow path — building those frames is the expensive work, and you have asked for them. (
DEBUG=1, which Jetpack tooling uses elsewhere, does not match this namespace and leaves the fast path in place.)