feat(minifier): minimize switch statements#17523
feat(minifier): minimize switch statements#17523armano2 wants to merge 36 commits intooxc-project:mainfrom
Conversation
Merging this PR will not alter performance
Comparing Footnotes
|
|
i'm pretty bad at naming stuff, suggestions are welcome 😄 |
| if switch_stmt.discriminant.may_have_side_effects(ctx) { | ||
| *stmt = ctx.ast.statement_expression( | ||
| switch_stmt.span, | ||
| switch_stmt.discriminant.take_in(ctx.ast), | ||
| ); | ||
| } else { | ||
| *stmt = ctx.ast.statement_empty(switch_stmt.span); | ||
| } |
There was a problem hiding this comment.
no need to change this, I'm just interested to learn.
Is it worth checking if the discrimimnant has side effects here?
If we were to assume it does have side effects, the other pass that visits the statement expression and checks if it does have side effects would remove it anyway?
I guess there's perhaps some careful reasoning here about duplicate checks on may_have_side_effects vs duplicate traversals
There was a problem hiding this comment.
correct, we do not want to keep expressions like switch(2){} -> 2
i' will wait for others to give feedback about this
camc314
left a comment
There was a problem hiding this comment.
This looks good to me! Thank you!
I'll let the experts (sapphi-red and Boshen have a look as well)
| } | ||
|
|
||
| // TODO: This is to aggressive, we should allow `break` for last elements in statements | ||
| impl<'a> Visit<'a> for FindNestedBreak { |
There was a problem hiding this comment.
this could be made more generic, and used within eg, for (; x; ) break; to check if loop could be completely in-lined,
maybe we could add something like is_statement_terminated or is_terminated to statement nodes, instead of my is_terminated_switch_case and with addition of FindNestedBreak that could be really useful
should i implement it in more generic way?
| /// non-empty case or case with side-effect-producing expressions backward to the last case. | ||
| /// - All cases in the identified removable suffix are eliminated, except for the last case, | ||
| /// which is preserved and its test is removed (if applicable). | ||
| fn collapse_empty_switch_cases(stmt: &mut Statement<'a>, ctx: &mut Ctx<'a, '_>) { |
There was a problem hiding this comment.
new optimization has been added to main, and this code is partially redundant, tho my code supports more use cases
eg, trailing empty switch cases,
oxc/crates/oxc_minifier/src/peephole/minimize_statements.rs
Lines 605 to 629 in 7242197
Phase 3: Advanced Optimizations
changes:
1-case and2-case switches into more compactiforif/elsestatements when safe.switch.note:
napilastbreakremoval from switch casesfuture improvements:
testhas no side effectsfixes #17544