You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
perf(javascript): escape strings, count name characters and find node tokens with less work - #23051
Three hot spots a CPU and allocation profile of the JavaScript minifier found, each with output byte for byte the same:
String literals: escaped in the loop that already counts their quotes, where a regexp called a function for every escape. A tab written raw is no longer escaped and then unescaped again.
Mangler character counts: the source is counted once, then adjusted for each number printed in decimal. Before, it copied the whole source up to three times.
A node's tokens: a node's start token is searched from the last start found. Before, it was searched from the parent's end, so it almost never hit the fast path.
Measured on lodash.js and three.module.js:
before
after
instructions per minify, three.js (callgrind, CodSpeed flags)
8,106.9M
8,043.1M (−0.79%)
instructions per minify, lodash
4,222.9M
4,205.9M (−0.40%)
allocated, lodash + three + acorn (sampling)
41.6 MB
38.6 MB (−7%)
token binary-search steps, three.js
511K
373K (−27%)
Output check: the minify corpora report is unchanged, and 402 node_modules files give the same output under three option sets, including ascii_only and ie8.
No new changeset: the pending "minifies faster" entry covers this.
What kind of change does this PR introduce?
perf
Did you add tests for your changes?
No. Output is unchanged, and the existing printer unit tests cover the changed lines (every escape, ie8, prefixed and separated numbers).
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
Claude Code profiled the minifier, made the changes and measured them (callgrind, allocation sampling, corpora and output comparisons). Reviewed by the author.
Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.
This PR includes no changesets
When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types
Reviewing files that changed from the base of the PR and between b7d8784 and c04a861.
Files selected for processing: 1
lib/javascript/syntax-printer.js
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 6 remain after this review.
Walkthrough
The syntax printer now handles string escapes during scanning, keeps separate search hints for token starts and ends, and counts decoded and normalized printed characters for mangled-name frequency.
Changes: Syntax printer behavior
Layer / File(s)
Summary
String escaping lib/javascript/syntax-printer.js
The string scan emits escapes as it processes qualifying characters. It handles NUL before a digit, vertical tabs in IE8 mode, and raw tabs according to the ASCII-only setting.
Token-start lookups use and update a separate saved hint from token-end lookups.
Printed-name frequency counting lib/javascript/syntax-printer.js
Frequency counting decodes valid Unicode escapes, counts supported radix-prefixed numbers by their printed decimal forms, and normalizes underscore-separated numbers.
Suggested labels:area: build-perf, needs: tests
Suggested reviewers:aryanraj45
Change: Refactor
Merge Risk: Minimal · up to c04a8
No merge-blocking issue was established in the printer changes.
Pre-merge checks: 4
Check name
Status
Explanation
Description Check
Check skipped - CodeRabbit’s high-level summary is enabled.
Title check
The title uses the conventional-commit form with type "perf" and scope "javascript". The change matches the type: the PR reduces minifier work through performance changes in string escaping, mangler c…
Linked Issues check
Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check
Check skipped because no linked issues were found for this pull request.
Full details: Title check
Explanation
The title uses the conventional-commit form with type "perf" and scope "javascript". The change matches the type: the PR reduces minifier work through performance changes in string escaping, mangler character counting, and token search. The title names the three areas the objectives describe. The title does not state the branch prefix, so the match to the branch prefix could not be checked from the provided context.
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Warning
Some tools did not complete. Review the errors below.
ast-grep (0.50.0)
lib/javascript/syntax-printer.js
ast-grep timed out on this file
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
… tokens with less work
A string literal is escaped in the pass that counts its quotes, where a
regexp called a function per escape, and a tab written raw is no longer
escaped to be unescaped again. The mangler counts the source's characters
once and adjusts for each number it prints in decimal, where it copied the
source up to three times. A node's start token is searched from the last
start found, where it was searched from its parent's end.
Claude-Session: https://claude.ai/code/session_01Tgxgyb4N35ndP6cC4KcrXC
Escape hatches count the raw types they stand for too: any, object and Function. Inferred any counts the identifiers whose type TypeScript infers as any though no JSDoc names it, and the new or generic calls whose type arguments it fills with any, such as const map = new Map().
This pull request adds or removes no escape-hatch type or inferred any.
120 file(s) in lib/, hot/ and declarations/ still use an escape hatch, infer `any` or have untyped lines
Comparing base (1b98b3e) to head (c04a861). Merging this pull request will not change the code webpack generates.
What moved
Tests
Assets
Gzip
Raw
Test edit
Changed, test untouched
0
0
—
—
—
Changed, test edited
0
0
—
—
—
New
0
0
—
—
—
Deleted
0
0
—
—
—
Unchanged
—
9870
—
—
—
Read Changed, test untouched first: the test is byte-identical on both sides, so webpack generated the difference. Changed, test edited moved partly because the test did — Test edit is how many bytes the test gained, and a bundle that grew by less is not a regression. New and Deleted are whole files, not deltas — a new asset was created, not grown, so it reads gray (⚪ added) — and adding tests cannot bury a real change. Gzip decides, because that is what users download; raw is the tiebreak, and brotli and zstd are per asset below.
Sample apps
No sample app changed size.
26 unchanged app(s)
App
Gzip
Gzip change
Raw
Raw change
library amd
1.38 KiB
—
2.68 KiB
—
library amd-require
1.38 KiB
—
2.67 KiB
—
library assign
1.38 KiB
—
2.67 KiB
—
library assign-properties
1.42 KiB
—
2.80 KiB
—
library commonjs
1.38 KiB
—
2.66 KiB
—
library commonjs-module
1.38 KiB
—
2.67 KiB
—
library commonjs-static
1.55 KiB
—
3.54 KiB
—
library commonjs2
1.38 KiB
—
2.67 KiB
—
library global
1.38 KiB
—
2.66 KiB
—
library jsonp
1.37 KiB
—
2.66 KiB
—
library module
1.21 KiB
—
2.35 KiB
—
library self
1.38 KiB
—
2.66 KiB
—
library system
1.41 KiB
—
2.73 KiB
—
library this
1.38 KiB
—
2.66 KiB
—
library umd
1.44 KiB
—
2.86 KiB
—
library umd2
1.44 KiB
—
2.86 KiB
—
library var
1.38 KiB
—
2.67 KiB
—
library window
1.38 KiB
—
2.66 KiB
—
node-ssr
96.36 KiB
—
371.30 KiB
—
node-ssr-esm
96.61 KiB
—
372.00 KiB
—
react-100
32.06 KiB
—
150.15 KiB
—
react-1k
73.05 KiB
—
906.51 KiB
—
react-1k-esm
72.54 KiB
—
906.44 KiB
—
react-5k
190.87 KiB
—
4.08 MiB
—
webworker
36.15 KiB
—
93.71 KiB
—
webworker-esm
35.67 KiB
—
94.21 KiB
—
No asset changed size.
No runtime gained or lost a runtime module.
Built test/configCases with the defaults a user gets, and the apps in test/sizeCoreReports (apps/): 2338 test(s), 9870 asset(s), 104 emitted nothing.
System calls cannot be simulated consistently and are left out of the measure. The reported value understates the real cost. Switch to the Walltime instrument to measure them.
⚠️1 benchmark with a significant change ran on different hardware
Its base and head results were measured on machines with a different CPU, core count or memory, so the change might not come from the code. Run base and head on the same runner type.
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
area: build-perfBuild time and memory of webpack itselfarea: parserJavaScript parsing and dependency analysis (lib/javascript, lib/dependencies)needs: testsFix or PR exists but lacks the required tests
1 participant
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.
github-actions
Bot
added
the
area: parser
coderabbitai
Bot
added
area: build-perf
Summary
Three hot spots a CPU and allocation profile of the JavaScript minifier found, each with output byte for byte the same:
Measured on
lodash.jsandthree.module.js:Output check: the minify corpora report is unchanged, and 402
node_modulesfiles give the same output under three option sets, includingascii_onlyandie8.No new changeset: the pending "minifies faster" entry covers this.
What kind of change does this PR introduce?
perf
Did you add tests for your changes?
No. Output is unchanged, and the existing printer unit tests cover the changed lines (every escape,
ie8, prefixed and separated numbers).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
Claude Code profiled the minifier, made the changes and measured them (callgrind, allocation sampling, corpora and output comparisons). Reviewed by the author.
Generated by Claude Code
Summary by CodeRabbit