Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf(es/minifier): Do not repeat applying pure minifier on last #10196

Merged
merged 8 commits into from
Mar 15, 2025

Conversation

kdy1
Copy link
Member

@kdy1 kdy1 commented Mar 14, 2025

Description:

This is mostly waste of time

@kdy1 kdy1 added this to the Planned milestone Mar 14, 2025
@kdy1 kdy1 self-assigned this Mar 14, 2025
Copy link

changeset-bot bot commented Mar 14, 2025

🦋 Changeset detected

Latest commit: 071205b

The changes in this PR will be included in the next version bump.

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

@@ -51,10 +51,12 @@ export default function(c, a) {
}
}(c);
if ("number" == typeof c && isFinite(c)) {
;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can someone help me fix the regression caused by this? We have

stmts.retain(|s| !matches!(s.as_stmt(), Some(Stmt::Empty(..))));
but it seems like this semicolon is created after invoking retain

cc @swc-project/core-es

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Understanding the traversal order remains challenging. A visualization tool that highlights AST node spans in the code editor during traversal, while logging corresponding visitor function locations, would be valuable. Implementing a step-through visualization that replays the traversal sequence within the editor interface could greatly enhance our debugging workflow.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let mut v = VarMover {
target,
vars: Default::default(),
var_decl_kind: Default::default(),
};
stmts.visit_mut_with(&mut v);

The key point is that the VarMover here will delve deeper into the AST to modify code at nested inner layers, while our upper-layer visitor currently operates at the outer level.

export default function(a, c) { // <- Pure::Visit_mut_stmts 
    try {
        if ("string" == typeof a && a.length > 0) return function(e) {
// ...
        }(a);
        if ("number" == typeof a && isFinite(a)) {
            var r, n, t, o;   // <- VarMover changed this
            return (null == c ? void 0 : c.long) ? (r = a, (n = Math.abs(r)) >= 86400000 ? s(r, n, 86400000, "day") : n >= 3600000 ? s(r, n, 3600000, "hour") : n >= 60000 ? s(r, n, 60000, "minute") : n >= 1000 ? s(r, n, 1000, "second") : "".concat(r, " ms")) : (t = a, (o = Math.abs(t)) >= 86400000 ? "".concat(Math.round(t / 86400000), "d") : o >= 3600000 ? "".concat(Math.round(t / 3600000), "h") : o >= 60000 ? "".concat(Math.round(t / 60000), "m") : o >= 1000 ? "".concat(Math.round(t / 1000), "s") : "".concat(t, "ms"));
        }
        throw Error("Value is not a string or number.");
    } catch (s) {
        var u; // <- VarMover changed this
        throw Error((void 0 === (u = s) ? "undefined" : e(u)) === "object" && null !== u && "message" in u ? "".concat(s.message, ". value=").concat(JSON.stringify(a)) : "An unknown error has occurred.");
    }
}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Understanding the traversal order remains challenging. A visualization tool that highlights AST node spans in the code editor during traversal, while logging corresponding visitor function locations, would be valuable. Implementing a step-through visualization that replays the traversal sequence within the editor interface could greatly enhance our debugging workflow.

Yeah, I love the idea, but I need to think about the way to implement it.


The key point is that the VarMover here will delve deeper into the AST to modify code at nested inner layers, while our upper-layer visitor currently operates at the outer level.

Yeap, thank you! I fixed it.

Copy link

codspeed-hq bot commented Mar 14, 2025

CodSpeed Performance Report

Merging #10196 will improve performances by 4.85%

Comparing kdy1/pass-min (7496601) with main (ec3ebe7)

Summary

⚡ 11 improvements
✅ 141 untouched benchmarks

Benchmarks breakdown

Benchmark BASE HEAD Change
es/minifier/libs/d3 497.6 ms 478.7 ms +3.96%
es/minifier/libs/echarts 1.9 s 1.9 s +3.49%
es/minifier/libs/jquery 128.3 ms 122.7 ms +4.57%
es/minifier/libs/lodash 147.4 ms 142.9 ms +3.11%
es/minifier/libs/moment 81.5 ms 78.2 ms +4.2%
es/minifier/libs/react 25.6 ms 24.6 ms +3.93%
es/minifier/libs/terser 447.7 ms 427 ms +4.85%
es/minifier/libs/three 805 ms 775.8 ms +3.76%
es/minifier/libs/typescript 4.8 s 4.6 s +3.72%
es/minifier/libs/victory 1.1 s 1.1 s +3.27%
es/minifier/libs/vue 187.6 ms 180.6 ms +3.85%

@kdy1 kdy1 marked this pull request as ready for review March 15, 2025 03:48
@kdy1 kdy1 requested a review from a team as a code owner March 15, 2025 03:48
kodiakhq[bot]
kodiakhq bot previously approved these changes Mar 15, 2025
@kdy1 kdy1 requested a review from a team as a code owner March 15, 2025 03:49
@kdy1 kdy1 enabled auto-merge (squash) March 15, 2025 03:49
@kdy1 kdy1 merged commit e6b7cee into main Mar 15, 2025
169 checks passed
@kdy1 kdy1 deleted the kdy1/pass-min branch March 15, 2025 04:19
@kdy1 kdy1 modified the milestones: Planned, v1.11.10 Mar 17, 2025
kdy1 added a commit to vercel/next.js that referenced this pull request Mar 17, 2025
### What?

Applies swc-project/swc#10196

### Why?

swc-project/swc#10196 makes minification 3%~4% faster
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants