Repository navigation
Consolidate acorn_optimizer calls in minify_wasm_js. NFC - #27928
Merged
Merged
Conversation
sbc100
force-pushed
the
consolidate-acorn-optimizer
branch
2 times, most recently
from
October 8, 2026 20:41
7875de7 to
13d8ad0
Compare
Previously, at `-O3`/`-Os`/`-Oz` (`expensive_optimizations`), `minify_wasm_js` invoked `acorn-optimizer.mjs` up to four separate times to parse and reprint JS: 1. Pre-`metadce` (`AJSDCE`, `stripDefaultUndefined`, `--minify-whitespace`) 2. Inside `metadce` (`applyDCEGraphRemovals`, `--minify-whitespace`) 3. Post-`metadce` (`AJSDCE`, `--minify-whitespace`) 4. Inside `minify_wasm_imports_and_exports` (`applyImportAndExportNameChanges`, `--minify-whitespace`) Have `metadce` and `minify_wasm_imports_and_exports` return their removal/renaming metadata instead of invoking `acorn_optimizer` directly, and consolidate `applyDCEGraphRemovals`, post-`metadce` `AJSDCE`, `applyImportAndExportNameChanges`, `stripDefaultUndefined`, and `--minify-whitespace` into a single final `acorn_optimizer` pass.
sbc100
force-pushed
the
consolidate-acorn-optimizer
branch
from
October 8, 2026 21:20
13d8ad0 to
171bccb
Compare
kripken
reviewed
Oct 8, 2026
| # now that we removed unneeded communication between js and wasm, we can | ||
| # clean up the js some more. | ||
| passes += ['applyDCEGraphRemovals', 'AJSDCE'] | ||
| extra_info.update(dce_info) |
Member
There was a problem hiding this comment.
why update()? extra_info is empty here, so we might as well do = dce_info - ?
Collaborator
Author
There was a problem hiding this comment.
I guess the idea is the extra_info can contains info for different passes at the same time. Yes, we know at this point in the code that no other pass has added extra info yet, but it seems good for future readers/editors if all changes to extra_info in this function addative and non-destructive.
(See below where minify_wasm_imports_and_exports adds to the extra_info).
kripken
approved these changes
Oct 8, 2026
| # now that we removed unneeded communication between js and wasm, we can | ||
| # clean up the js some more. | ||
| passes += ['applyDCEGraphRemovals', 'AJSDCE'] | ||
| extra_info.update(dce_info) |
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
2 participants
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.
Previously, at
-O3/-Os/-Oz(expensive_optimizations),minify_wasm_jsinvokedacorn-optimizer.mjsup to four separate times to parse and reprint JS:metadce(AJSDCE,stripDefaultUndefined,--minify-whitespace)metadce(applyDCEGraphRemovals,--minify-whitespace)metadce(AJSDCE,--minify-whitespace)minify_wasm_imports_and_exports(applyImportAndExportNameChanges,--minify-whitespace)Have
metadceandminify_wasm_imports_and_exportsreturn their removal/renaming metadata instead of invokingacorn_optimizerdirectly, and consolidateapplyDCEGraphRemovals, post-metadceAJSDCE,applyImportAndExportNameChanges,stripDefaultUndefined, and--minify-whitespaceinto a single finalacorn_optimizerpass.Split out from #27926