perf: reduce allocations and speed up codegen hot paths for harmony/commonjs dependencies#21180
Conversation
- processExportInfo: allocate the visited Set lazily (only on recursion) instead of on every call via a default parameter, and drop the redundant add/delete on the terminal path. - chainedImports trimIdsToThoseImported: avoid the eager empty-array that was always discarded. - HarmonyExportInitFragment.getContent: skip the entries spread + sort when there are no exports, and skip sorting a single entry. - HarmonyImportDependencyParserPlugin: share one empty idRanges array for plain specifier references instead of allocating per dependency.
- Template.toComment/toNormalComment: skip the COMMENT_END regex replace when the string contains no occurrence of the comment terminator. - util/property propertyAccess: only run the Number() round-trip for properties whose first char can start a canonical number/NaN/Infinity, avoiding a Number() call and string allocation for ordinary identifiers. - ExportInfo.getUsedName: replace the spread+every() over the runtime set with a short-circuiting loop to avoid an array allocation per export.
🦋 Changeset detectedLatest commit: feba074 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
This PR is packaged and the instant preview is available (914c7ec). Install it locally:
npm i -D webpack@https://pkg.pr.new/webpack@914c7ec
yarn add -D webpack@https://pkg.pr.new/webpack@914c7ec
pnpm add -D webpack@https://pkg.pr.new/webpack@914c7ec |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #21180 +/- ##
==========================================
- Coverage 92.68% 92.65% -0.03%
==========================================
Files 587 587
Lines 63962 63968 +6
Branches 17726 17732 +6
==========================================
- Hits 59280 59270 -10
- Misses 4682 4698 +16
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Merging this PR will degrade performance by 61.87%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Memory | benchmark "lodash", scenario '{"name":"mode-development-rebuild","mode":"development","watch":true}' |
126.1 KB | 859.4 KB | -85.33% |
| ❌ | Memory | benchmark "asset-modules-inline", scenario '{"name":"mode-development-rebuild","mode":"development","watch":true}' |
317.3 KB | 1,299.8 KB | -75.59% |
| ❌ | Memory | benchmark "css-modules", scenario '{"name":"mode-production","mode":"production"}' |
7.2 MB | 9.4 MB | -23.24% |
| ❌ | Memory | benchmark "asset-modules-bytes", scenario '{"name":"mode-development-rebuild","mode":"development","watch":true}' |
247.2 KB | 321.6 KB | -23.13% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing claude/harmony-commonjs-perf-stdhhl (feba074) with main (0bf661b)
Footnotes
-
36 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
Summary
Several constant-factor wins on per-module / per-dependency / per-export hot paths used by harmony and commonjs code generation, all output-identical (no change to emitted bundles):
processExportInfo: allocate thealreadyVisitedSetlazily (only when recursing) instead of via a default arg on every call, and drop the redundantadd/deleteon the terminal path.util/chainedImportstrimIdsToThoseImported: drop the eager empty array that was always discarded.HarmonyExportInitFragment.getContent: skip the entries spread + sort when there are no exports, and skip sorting a single entry.HarmonyImportDependencyParserPlugin: share one (never-mutated) emptyidRangesarray for plain specifier references instead of allocating per dependency.Template.toComment/toNormalComment: skip the comment-terminator regex replace when the string contains no*/.util/propertypropertyAccess: only run theNumber()round-trip for properties whose first char can start a canonical number/NaN/Infinity, avoiding aNumber()call and string allocation for ordinary identifiers.ExportInfo.getUsedName: replace[...runtime].every(...)with a short-circuiting loop to avoid allocating an array from the runtime set per export.Micro-benchmarks of the affected functions show ~1.25x–10x speedups on their common paths; aggregate build-time impact is within run-to-run noise but allocation/GC pressure is reduced.
What kind of change does this PR introduce?
perf
Did you add tests for your changes?
No new tests — these are behavior-preserving optimizations with byte-identical output, covered by the existing
StatsTestCasessnapshots andTestCases/ConfigTestCases(verified passing, incl. tree-shaking, mangle, runtime, concatenation, split-chunks).propertyAccessand the comment helpers were additionally diffed against the previous implementations over edge cases.Does this PR introduce a breaking change?
No.
If relevant, what needs to be documented once your changes are merged or what have you already documented?
n/a
Use of AI
AI (Claude) was used to survey the dependency/codegen hot paths, propose candidate optimizations, and draft the changes; every change was human-reviewed and verified for output-equivalence against existing tests and direct old-vs-new comparisons before inclusion.
Generated by Claude Code