fix(core): shrink thin-engine bundle (remove fileTools force-link + thin-closure stubs)#18656
fix(core): shrink thin-engine bundle (remove fileTools force-link + thin-closure stubs)#18656RaananW wants to merge 5 commits into
Conversation
…hin-closure stubs) The pure-barrel refactor (#18441) regressed the thin-engine bundle by force-linking fileTools into the ThinEngine closure and by emitting side-effect stubs into thin-engine files. fileTools de-linking (restores lazy injection seams): - abstractEngine: statics route through EngineFunctionContext (loadImage added); _LoadFile throws _WarnImport when no loader is injected. - shaderProcessor.ProcessIncludes: call _FunctionContainer.loadFile (throwing stub) instead of importing LoadFile from fileTools.pure. - RegisterFileTools() now injects EngineFunctionContext.loadFile/.loadImage and _FunctionContainer.loadFile. Stub generator: - Automatically exclude the thinEngine.pure import closure (computed via esbuild) from side-effect stub emission, so Effect/Observable and other thin-closure files no longer carry stubs. Heavier engine variants keep their stubs via STUB_EXCLUDED_CLASSES. Production ES6 build (constants inlined), thin-engine consumer bundle: min: 178,059 -> 152,584 (-25,475, -14.3%) gzip: 44,794 -> 37,781 ( -7,013, -15.7%) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s). |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
|
Snapshot stored with reference name: Test environment: To test a playground add it to the URL, for example: https://snapshots-cvgtc2eugrd3cgfd.z01.azurefd.net/refs/pull/18656/merge/index.html#WGZLGJ#4600 Links to test your changes to core in the published versions of the Babylon tools (does not contain changes you made to the tools themselves): https://playground.babylonjs.com/?snapshot=refs/pull/18656/merge To test the snapshot in the playground with a playground ID add it after the snapshot query string: https://playground.babylonjs.com/?snapshot=refs/pull/18656/merge#BCU1XR#0 If you made changes to the sandbox or playground in this PR, additional comments will be generated soon containing links to the dev versions of those tools. |
🟢 Memory Leak Test Results4 passed, 0 leaked out of 4 scenarios 🟢 All memory leak tests passed — no leaks detected. Passed Scenarios (4)
|
|
Visualization tests for WebGPU |
|
WebGL2 visualization test reporter: |
⚡ Performance Test Results🟢 All performance tests passed — no regressions detected. |
…ne closure The previous approach computed the thin-engine closure by bundling thinEngine.pure.ts with esbuild from source. That under-counted the real closure by 12 modules (constants inlining + injected /*#__PURE__*/ annotations make the shipped build tree-shake differently than a source-only pass), so a stub still leaked into Buffers/buffer.pure.ts (VertexBuffer.effectiveByte*). Replace the computed closure with an authoritative, committed list (scripts/treeshaking/thin-engine-modules.json) of the 53 core modules pulled in by `new ThinEngine(canvas)` (packages/tools/tests/src/thinEngineOnly.ts). The stub generator now: - loads the list and validates every module exists (a rename can never silently drop a file out of the exclusion set), - excludes all of them from stub emission, - drops the esbuild dependency and the async closure computation. Result: buffer.pure VertexBuffer stub removed; all 53 thin-engine modules are guaranteed stub-free, enforced by `generate:side-effect-stubs --check` in CI. Production thin-engine bundle size is unchanged from the prior commit (178,059 -> 152,584 min; 44,794 -> 37,781 gzip). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
The `new Set()` initializer was always overwritten by loadThinEngineClosure() before any read. Declare STUB_EXCLUDED_FILES uninitialized (assigned once at module load, before Step 2 uses it) to remove the useless assignment. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
🟢 Memory Leak Test Results4 passed, 0 leaked out of 4 scenarios 🟢 All memory leak tests passed — no leaks detected. Passed Scenarios (4)
|
|
Visualization tests for WebGPU |
|
WebGL2 visualization test reporter: |
…egistered
The thin-engine size work de-linked fileTools from the always-loaded engine
base classes and made LoadFile/LoadImage lazily injected via
EngineFunctionContext. The full-index (barrel) build still imports the
fileTools side effect, but deep imports ("@babylonjs/core/Engines/engine")
and pure imports (RegisterStandardEngineExtensions) no longer registered it,
so any scene that loaded an external asset threw _WarnImport("FileTools")
and never became ready. This broke the ES6 deep/pure visualization tests.
RegisterAbstractEngineLoadFile() is reached by both the full Engine
side-effect wrapper and RegisterStandardEngineExtensions, but not by the
minimal thin engine. Registering fileTools there restores file/image loading
for deep and pure consumers while keeping the thin engine free of fileTools.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
🟢 Memory Leak Test Results4 passed, 0 leaked out of 4 scenarios 🟢 All memory leak tests passed — no leaks detected. Passed Scenarios (4)
|
|
WebGL2 visualization test reporter: |
|
Visualization tests for WebGPU |
⚡ Performance Test Results🟢 All performance tests passed — no regressions detected. |
|
Let s also kill the flags.json override at 20% in the repo ? and maybe lock down the thinEngineOnlyTest to 2k growth ? |
…th at 2KB/PR - Lower the global package-size error threshold from 20% to 10% (.build/flags.json errorThreshold 1.2 -> 1.1). - Add an absolute per-case growth cap in generateFileSizes.js: the thin engine (thinEngineOnly) may not grow by more than 2 KB in a single PR, regardless of its percentage headroom. This keeps the minimal thin-engine bundle from creeping up over time. - Fix the threshold comparison to use the resolved loadedMainSize instead of the raw CDN entry (which is an object for the new main/async format), so the percentage check is correct for both legacy and current size entries. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
🟢 Memory Leak Test Results4 passed, 0 leaked out of 4 scenarios 🟢 All memory leak tests passed — no leaks detected. Passed Scenarios (4)
|
|
Visualization tests for WebGPU |
|
WebGL2 visualization test reporter: |
|
@sebavan Done |
⚡ Performance Test Results🟢 All performance tests passed — no regressions detected. |
There was a problem hiding this comment.
Pull request overview
Reduces ThinEngine consumer bundle size by restoring lazy file/image-loading injection seams (so fileTools is not force-linked into the thin-engine import closure) and by preventing side-effect stub emission into always-loaded thin-engine closure files.
Changes:
- Reworked file/image loading to route through
EngineFunctionContextand_FunctionContainer, throwing_WarnImport("FileTools")when loaders aren’t registered (instead of importingfileToolsimplementations). - Updated the side-effect stub generator to exclude the thin-engine closure (using an authoritative module list) and to strip stale stub regions/imports cleanly.
- Tightened bundle-size CI enforcement for the thin-engine case (absolute growth cap + stricter default error threshold).
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/treeshaking/thin-engine-modules.json | Adds authoritative thin-engine import-closure module list used to exclude files from stub emission. |
| scripts/treeshaking/generateSideEffectStubs.mjs | Excludes thin-engine closure + always-loaded engine classes from stub generation; improves stale-stub removal and import cleanup. |
| packages/tools/tests/scripts/generateFileSizes.js | Adds an absolute per-PR growth cap for thinEngineOnly and fixes threshold comparisons to use loaded baseline sizes. |
| .build/flags.json | Tightens default errorThreshold to 1.1 for bundle-size checks. |
| packages/dev/core/src/Engines/abstractEngine.functions.ts | Removes fileTools fallback; _LoadFile now relies on injected loader or throws _WarnImport("FileTools"); adds loadImage seam. |
| packages/dev/core/src/Engines/abstractEngine.pure.ts | Routes file/image loading through EngineFunctionContext and throws when loaders aren’t registered; removes stub region. |
| packages/dev/core/src/Engines/Processors/shaderProcessor.ts | Removes LoadFile import; include-loading now uses _FunctionContainer.loadFile stub/injection seam. |
| packages/dev/core/src/Misc/fileTools.pure.ts | RegisterFileTools() now injects loadFile/loadImage into EngineFunctionContext and wires _FunctionContainer.loadFile. |
| packages/dev/core/src/Engines/AbstractEngine/abstractEngine.loadFile.pure.ts | Ensures RegisterFileTools() runs when registering the engine load-file extension (so full engines still wire loaders). |
| packages/dev/core/src/Engines/thinEngine.pure.ts | Removes generated side-effect stub region from thin-engine closure file. |
| packages/dev/core/src/Engines/engine.pure.ts | Removes generated side-effect stub region from always-loaded Engine class file. |
| packages/dev/core/src/Materials/effect.pure.ts | Removes generated side-effect stub region from thin-engine closure file. |
| packages/dev/core/src/Misc/observable.pure.ts | Removes generated side-effect stub region from thin-engine closure file. |
| packages/dev/core/src/Buffers/buffer.pure.ts | Removes generated side-effect stub region from thin-engine closure file. |
🟢 Memory Leak Test Results4 passed, 0 leaked out of 4 scenarios 🟢 All memory leak tests passed — no leaks detected. Passed Scenarios (4)
|
|
Visualization tests for WebGPU |
|
WebGL2 visualization test reporter: |
⚡ Performance Test Results🟢 All performance tests passed — no regressions detected. |
|
Let s merge Monday so we do not have to monitor tomorrow :-) |
Problem
The pure-barrel tree-shaking refactor (#18441) regressed the thin-engine bundle size. Two root causes:
fileToolswas force-linked into theThinEngineimport closure. Pure-barrel inlined the lazy-injection seams that previously keptfileToolsout of a thin bundle, so a thin-engine-only consumer pulled the full file/image loading code even when unused.Effect,Observable), adding dead weight to the minimal closure.The thin engine is meant to be the smallest possible entry point, so both regressions matter.
Changes
fileToolsde-linking — restore lazy injection seamsabstractEngine: statics route throughEngineFunctionContext(addedloadImage);_LoadFilethrows_WarnImport("FileTools")when no loader is injected instead of importing the implementation.shaderProcessor.ProcessIncludes: calls the throwing stub_FunctionContainer.loadFileinstead of importingLoadFilefromfileTools.pure(this was the third, previously-missed seam).RegisterFileTools()now injectsEngineFunctionContext.loadFile/.loadImageand_FunctionContainer.loadFile, so the real implementation is wired up only whenfileToolsis actually imported.Stub generator — auto-exclude the thin-engine closure
scripts/treeshaking/generateSideEffectStubs.mjsnow computes thethinEngine.pureimport closure (via esbuild) and excludes those files from side-effect stub emission.Effect/Observableand other thin-closure files no longer carry stubs. Heavier engine variants (Engine/WebGPU/Native) still keep their stubs viaSTUB_EXCLUDED_CLASSES.Note on
ConstantsAn earlier source-based measurement suggested the
Constantsgod-object (~15 KB) was a further lever. It is not —prepareEs6Build.tsalready inlines everyConstants.Xto a literal and strips the import during the shipped ES6 build (prepare-es6-build -cf Engines/constants.js), soconstants.jsis dropped from real bundles. NoConstantschange was needed, and the numbers below are measured against the real inlined production build.Results (real production ES6 build, constants inlined +
/*#__PURE__*/annotations, minified)Thin-engine consumer bundle (
import { ThinEngine } from "@babylonjs/core/Engines/thinEngine"), bundled with esbuild--minify:Validation
tsc -b(core production build) ✅verifyTreeShaking --all-packages— 16/16 ✅ (manifest drift, side-effect import closure, pure barrels, side-effect stubs across core/gui/loaders/serializers)generate:side-effect-stubs --check) ✅Public API is unchanged; behavior is preserved because the file/image loaders are still wired up whenever
fileToolsis imported (as before the regression).Opening as draft to let CI run the full suite.