refactor: stabilize the NormalModule instance shape - #21515
Conversation
🦋 Changeset detectedLatest commit: ec0a161 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 (8331267). Install it locally:
npm i -D webpack@https://pkg.pr.new/webpack@8331267
yarn add -D webpack@https://pkg.pr.new/webpack@8331267
pnpm add -D webpack@https://pkg.pr.new/webpack@8331267 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #21515 +/- ##
==========================================
+ Coverage 93.60% 93.78% +0.18%
==========================================
Files 619 620 +1
Lines 73650 73704 +54
Branches 21244 21262 +18
==========================================
+ Hits 68939 69124 +185
+ Misses 4711 4580 -131
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 1.49%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Memory | benchmark "asset-modules-resource", scenario '{"name":"mode-development-rebuild","mode":"development","watch":true}' |
632.9 KB | 1,207.3 KB | -47.58% |
| ❌ | Memory | benchmark "cache-filesystem", scenario '{"name":"mode-production","mode":"production"}' |
3.9 MB | 4.9 MB | -20.39% |
| ⚡ | Memory | benchmark "asset-modules-source", scenario '{"name":"mode-development-rebuild","mode":"development","watch":true}' |
1,335 KB | 750.3 KB | +77.92% |
| ⚡ | Memory | benchmark "many-chunks-commonjs", scenario '{"name":"mode-production","mode":"production"}' |
9.3 MB | 7.3 MB | +26.84% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing claude/dynamic-properties-hidden-classes-0ebn71 (ec0a161) with main (9d01716)
Initialize _ast in the constructor instead of adding it during build, so every NormalModule keeps a single V8 hidden-class shape from creation rather than transitioning at build time (which also removes a wrong-map deopt on the per-module read path). Behavior is unchanged: _ast is only read via `this._ast || source`. The field is private, so types.d.ts is unaffected. Consistency/hygiene change, not a measured speedup.
Add a Performance guideline to initialize every instance field in the constructor so instances stay on one V8 hidden-class shape, avoiding polymorphic/megamorphic inline caches and wrong-map deopts.
a33a14d to
ec0a161
Compare
Types CoverageCoverage after merging claude/dynamic-properties-hidden-classes-0ebn71 into main will be
Coverage Report |
Summary
webpack keeps V8 hidden-class (Shape) transitions stable by initializing every instance field in the constructor (e.g.
Dependency's_loc*slots), butNormalModule._astwas still added after construction (assigned in_doBuild/build). Declaring it in the constructor keeps eachNormalModuleon a single shape from creation and removes awrong mapdeopt on the per-module read path._astis private, sotypes.d.tsis unaffected. This is a consistency/hygiene change — an isolated micro-benchmark shows the late-added field spilled to an out-of-line property array (constructor-init objects are ~8 B/instance lighter and construct no slower), but on a full 4000-module build the difference stays inside run-to-run noise, so it is intentionally submitted asrefactor, notperf.What kind of change does this PR introduce?
refactor
Did you add tests for your changes?
No — the change only moves field initialization into the constructor and does not alter behavior:
_astis read only viathis._ast || source, and no code doesin/hasOwnProperty/=== undefinedon it, so the pre-initializednulldefault is indistinguishable from the previous absent-property read. ExistingNormalModulecoverage applies.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
Yes. Claude (Claude Code) was used to scan the core classes (
Module,NormalModule,Dependency+ subclasses,Chunk,ChunkGraph,ModuleGraph,Compilation, …) for fields assigned after construction, to write the constructor initialization, and to run the benchmarks that confirmed the change is behavior- and build-time-neutral. All changes were reviewed by the author before submitting.