Repository navigation
Reduce devtools extension build size via safe engine-asset pruning - #10063
NemesisDevX wants to merge 4 commits into
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
`flutter build web` copies every built web engine artifact into build/web/canvaskit, so the packaged extension/devtools/build output is ~44 MB even though a given build never loads most of those files at runtime (flutter#9897). This change makes build_and_copy prune what is provably or explicitly unneeded from the copied destination (source build output is untouched): - .js.symbols debug files are always removed; the bootstrap loader never fetches them. - skwasm/wimp renderer files are removed only when the build output contains no .wasm application artifact at its root, which proves the wasm renderer cannot be selected independently of any runtime configuration. - A new opt-in --prune-cdn-canvaskit flag removes the entire canvaskit/ directory for extensions whose owners can assert the default CDN configuration applies (no custom bootstrap, index.html initialization or runtime config overrides). It is rejected when combined with --no-web-resources-cdn. The default remains conservative: whether a build loads CanvasKit from the CDN depends on mutable runtime configuration that cannot be proven statically, so full pruning requires explicit opt-in. Measured on the example standalone extension (flutter build web): default CDN build 44 -> 23 MB (or 7 MB with --prune-cdn-canvaskit), offline 23 MB, --tree-shake-icons 21 MB. Assisted-by: Devin AI
0402666 to
8ed122f
Compare
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces size-reduction features for DevTools extensions by pruning unused web engine resources during the build process. It adds new flags (--tree-shake-icons, --web-resources-cdn, and --prune-cdn-canvaskit) to the build_and_copy command, implements the pruning logic in _build_and_copy.dart, and includes comprehensive unit tests. Feedback on the changes suggests optimizing the pruneCopiedBuildOutput method to avoid redundant directory listings and file deletions when pruneCdnCanvasKit is enabled.
| var prunedBytes = 0; | ||
|
|
||
| // Debug symbols are never fetched at runtime in any build mode. | ||
| for (final entity in canvaskitDir.listSync(recursive: true)) { | ||
| if (entity is File && entity.path.endsWith('.js.symbols')) { | ||
| prunedBytes += entity.lengthSync(); | ||
| entity.deleteSync(); | ||
| } | ||
| } | ||
|
|
||
| if (pruneCdnCanvasKit) { | ||
| // Opt-in aggressive prune: the caller asserts every engine resource | ||
| // is loaded remotely at runtime. | ||
| for (final entity in canvaskitDir.listSync(recursive: true)) { | ||
| if (entity is File) prunedBytes += entity.lengthSync(); | ||
| } | ||
| canvaskitDir.deleteSync(recursive: true); | ||
| } else if (!Directory( | ||
| destinationBuildPath, | ||
| ).listSync().whereType<File>().any((file) => file.path.endsWith('.wasm'))) { | ||
| // No wasm application artifact exists in this output, so no | ||
| // configuration can select the wasm renderer and its engine files | ||
| // are unreachable. The local `canvaskit/`, `chromium/` and | ||
| // `webparagraph/` variants are kept because the loader selects | ||
| // between them at runtime based on browser support. | ||
| for (final entity in canvaskitDir.listSync()) { | ||
| if (entity is! File) continue; | ||
| final name = path.basename(entity.path); | ||
| if (name.startsWith('skwasm') || name.startsWith('wimp')) { | ||
| prunedBytes += entity.lengthSync(); | ||
| entity.deleteSync(); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
[CONCERN] The current implementation performs redundant directory listings and file deletions when pruneCdnCanvasKit is enabled.\n\nSpecifically:\n1. It always performs a recursive directory listing to delete .js.symbols files first, even if pruneCdnCanvasKit is true and the entire canvaskit directory is about to be deleted recursively anyway.\n2. If pruneCdnCanvasKit is true, it performs a second recursive directory listing to sum up the remaining file sizes, and then deletes the directory recursively.\n\nWe can optimize this by restructuring the logic to handle the pruneCdnCanvasKit case first. This avoids the redundant recursive listing and individual file deletions.
var prunedBytes = 0;\n\n if (pruneCdnCanvasKit) {\n // Opt-in aggressive prune: the caller asserts every engine resource\n // is loaded remotely at runtime.\n for (final entity in canvaskitDir.listSync(recursive: true)) {\n if (entity is File) {\n prunedBytes += entity.lengthSync();\n }\n }\n canvaskitDir.deleteSync(recursive: true);\n } else {\n // Debug symbols are never fetched at runtime in any build mode.\n for (final entity in canvaskitDir.listSync(recursive: true)) {\n if (entity is File && entity.path.endsWith('.js.symbols')) {\n prunedBytes += entity.lengthSync();\n entity.deleteSync();\n }\n }\n\n if (!Directory(destinationBuildPath)\n .listSync()\n .whereType<File>()\n .any((file) => file.path.endsWith('.wasm'))) {\n // No wasm application artifact exists in this output, so no\n // configuration can select the wasm renderer and its engine files\n // are unreachable. The local `canvaskit/`, `chromium/` and\n // `webparagraph/` variants are kept because the loader selects\n // between them at runtime based on browser support.\n for (final entity in canvaskitDir.listSync()) {\n if (entity is! File) continue;\n final name = path.basename(entity.path);\n if (name.startsWith('skwasm') || name.startsWith('wimp')) {\n prunedBytes += entity.lengthSync();\n entity.deleteSync();\n }\n }\n }\n }References
- Avoid repeated sub-expressions or function calls within the same method to improve efficiency and maintainability. (link)
There was a problem hiding this comment.
Addressed in 03603711 (with a dart format follow-up in 2bb30c0, current head): the opt-in path now performs a single recursive listSync to sum file sizes before deleting canvaskit/ outright, and the symbol/wasm pruning logic only runs on the conservative default path.
Verification on the new head (2bb30c0): dart test build_and_copy 17/17, validate 4/4, flutter test api 12/12, dart analyze/dart format clean, dt presubmit passed. Fresh end-to-end builds on the example extension: default CDN 44 -> 23 MB (symbols + unreachable wasm pruned, runtime set kept), --prune-cdn-canvaskit 44 -> 7 MB (directory removed).
One note for maintainers: this change affects the devtools_extensions build-and-copy CLI rather than the DevTools app UI, so I've left the release-notes checklist box unchecked — guidance welcome on whether release-notes-not-required (or another label) is appropriate here. Happy to adjust.
Assisted-by: GPT-6
Assisted-by: GPT-6
dart format reflows the root-level .wasm artifact check introduced in 0360371; no behavior change. Assisted-by: Devin AI
Fixes #9897
flutter build webcopies every built web engine artifact intobuild/web/canvaskit, so the packagedextension/devtools/buildoutput is ~44 MB even though a given extension never loads most of those files at runtime.This PR makes
build_and_copyprune what is provably or explicitly unneeded from the copied destination only (the sourcebuild/weboutput is untouched):.js.symbolsdebug files are always removed — the bootstrap loader never fetches them.skwasm/wimprenderer files are removed only when the build output contains no.wasmapplication artifact at its root.flutter build webemitsmain.dart.wasmonly fordart2wasmbuilds, so its absence proves the wasm renderer cannot be selected — independent of any runtime configuration.--prune-cdn-canvaskitflag removes the entirecanvaskit/directory for extensions whose owners can assert the default CDN configuration applies (no custom bootstrap,index.htmlinitialization, or runtime configuration overrides). It is rejected when combined with--no-web-resources-cdn.Full pruning is deliberately opt-in: whether a build loads CanvasKit from the CDN depends on mutable runtime configuration (bootstrap reassignment/mutation,
load({config: ...}),index.htmlinitialization) that cannot be proven statically, so the default stays conservative in every mode.Measured on the example
standalone_extension(freshflutter build web):--prune-cdn-canvaskit--tree-shake-icons--no-web-resources-cdnTesting performed on the original head
8ed122f:dart test17/17 forbuild_and_copy(incl. regression fixtures for runtime-unsafe bootstrap configurations and the rejected flag combination),dart testvalidate 4/4,flutter testapi 12/12,dart analyze/dart format/git diff --checkclean. The size table above was measured on that head; measurements were re-confirmed on the current head (below).Follow-up commits (addressing review feedback)
0360371— avoids the redundant recursive listing and per-file deletions when--prune-cdn-canvaskitis set (one listing to sum sizes, then delete the directory). Co-authored with AI review tooling.cf0fe41— changelog wording now covers any root-level.wasmapplication artifact, including content-hashed filenames.2bb30c0—dart formatfix-up on the new condition.Verification on current head
2bb30c0:dart testbuild_and_copy 17/17,dart testvalidate 4/4,flutter testapi 12/12,dart analyze/dart formatclean,dt presubmitpassed, and fresh end-to-end builds reproduce the same sizes as the table (default CDN 23 MB,--prune-cdn-canvaskit7 MB). Not run: browser-hostedtest/web/suite.Parts of this PR (including commits
0360371/cf0fe41) were assisted by AI review tooling; all changes were reviewed and verified by me before landing.Pre-launch Checklist
General checklist
///).Issues checklist
contributions-welcomeorgood-first-issuelabel. I understand this means my PR might take longer to be reviewed.Tests checklist
AI-tooling checklist
Feature-change checklist
release-notes-not-requiredlabel or left a comment requesting the label be added.