From 934c75c7f06312aa7a8f23cb9d2da9043b56f402 Mon Sep 17 00:00:00 2001 From: Cameron Clark Date: Wed, 15 Jan 2025 14:33:22 +0000 Subject: [PATCH] feat(minifier): merge similar if stmts --- .../peephole_minimize_conditions.rs | 61 ++++++++++--- crates/oxc_minifier/tests/ast_passes/mod.rs | 4 +- tasks/coverage/src/tools/esbuild.rs | 85 +++++++++++++++++++ tasks/minsize/minsize.snap | 20 ++--- 4 files changed, 147 insertions(+), 23 deletions(-) create mode 100644 tasks/coverage/src/tools/esbuild.rs diff --git a/crates/oxc_minifier/src/ast_passes/peephole_minimize_conditions.rs b/crates/oxc_minifier/src/ast_passes/peephole_minimize_conditions.rs index 53738d92b7267..4032885fac1a0 100644 --- a/crates/oxc_minifier/src/ast_passes/peephole_minimize_conditions.rs +++ b/crates/oxc_minifier/src/ast_passes/peephole_minimize_conditions.rs @@ -215,19 +215,53 @@ impl<'a> PeepholeMinimizeConditions { fn try_replace_if(&mut self, stmts: &mut Vec<'a, Statement<'a>>, ctx: &mut TraverseCtx<'a>) { for i in 0..stmts.len() { - let Statement::IfStatement(if_stmt) = &stmts[i] else { + let (current_node, next_node) = { + if i == stmts.len() - 1 { + (stmts.get_mut(i).unwrap(), None) + } else { + let (left, right) = stmts.split_at_mut(i + 1); + (left.last_mut().unwrap(), right.first_mut()) + } + }; + + let Statement::IfStatement(if_stmt) = ¤t_node else { continue; }; + let then_branch = &if_stmt.consequent; let else_branch = &if_stmt.alternate; - let next_node = stmts.get(i + 1); - if next_node.is_some_and(|s| matches!(s, Statement::IfStatement(_))) + if next_node.as_ref().is_some_and(|s| matches!(s, Statement::IfStatement(_))) && else_branch.is_none() - && Self::is_return_block(then_branch) + && Self::statement_must_exit_parent(then_branch) { - /* TODO */ - } else if next_node.is_some_and(Self::is_return_expression) + let next_node = next_node.unwrap(); + // `if (x) return; if (y) return;` -> `if (x || y) return;` + let Statement::IfStatement(next_if) = next_node else { unreachable!() }; + + if next_if.alternate.is_none() && next_if.consequent.content_eq(&if_stmt.consequent) + { + let Statement::IfStatement(mut if_stmt) = ctx.ast.move_statement(current_node) + else { + unreachable!() + }; + + let Statement::IfStatement(mut next) = ctx.ast.move_statement(next_node) else { + unreachable!() + }; + let test = ctx.ast.expression_logical( + next.test.span(), + ctx.ast.move_expression(&mut if_stmt.test), + LogicalOperator::Or, + ctx.ast.move_expression(&mut next.test), + ); + if_stmt.test = test; + stmts[i] = Statement::IfStatement(if_stmt); + self.changed = true; + } + + // TODO: `if (x) return 1; if (y) foo() else return 1;` -> `if (!x&&y) foo() else return 1;` + } else if next_node.as_deref().is_some_and(Self::is_return_expression) && else_branch.is_none() && Self::is_return_block(then_branch) { @@ -248,6 +282,7 @@ impl<'a> PeepholeMinimizeConditions { self.changed = true; break; } else if else_branch.is_some() && Self::statement_must_exit_parent(then_branch) { + // `if (x) return; else return 1` -> `return x ? void 0 : 1` let Statement::IfStatement(if_stmt) = &mut stmts[i] else { unreachable!(); }; @@ -951,12 +986,16 @@ mod test { } #[test] - #[ignore] - fn test_combine_ifs1() { + fn test_combine_ifs() { fold( "function f() {if (x) return 1; if (y) return 1}", "function f() {if (x||y) return 1;}", ); + } + + #[test] + #[ignore] + fn test_combine_ifs1() { fold( "function f() {if (x) return 1; if (y) foo(); else return 1}", "function f() {if ((!x)&&y) foo(); else return 1;}", @@ -966,8 +1005,10 @@ mod test { #[test] #[ignore] fn test_combine_ifs2() { - // combinable but not yet done - fold_same("function f() {if (x) throw 1; if (y) throw 1}"); + fold( + "function f() {if (x) throw 1; if (y) throw 1}", + "function f() {if (x || y) throw 1;}", + ); // Can't combine, side-effect fold("function f(){ if (x) g(); if (y) g() }", "function f(){ x&&g(); y&&g() }"); fold("function f(){ if (x) g?.(); if (y) g?.() }", "function f(){ x&&g?.(); y&&g?.() }"); diff --git a/crates/oxc_minifier/tests/ast_passes/mod.rs b/crates/oxc_minifier/tests/ast_passes/mod.rs index ca203b08550fe..da9fdc36cd9f8 100644 --- a/crates/oxc_minifier/tests/ast_passes/mod.rs +++ b/crates/oxc_minifier/tests/ast_passes/mod.rs @@ -149,9 +149,7 @@ fn cjs() { });"#, " Object.keys(_index6).forEach(function(key) { - if (key === 'default' || key === '__esModule') return; - if (Object.prototype.hasOwnProperty.call(_exportNames, key)) return; - if (key in exports && exports[key] === _index6[key]) return; + if (key === 'default' || key === '__esModule' || Object.prototype.hasOwnProperty.call(_exportNames, key) || key in exports && exports[key] === _index6[key]) return; Object.defineProperty(exports, key, { enumerable: true, get: function() { diff --git a/tasks/coverage/src/tools/esbuild.rs b/tasks/coverage/src/tools/esbuild.rs new file mode 100644 index 0000000000000..fa33c4e76a0d1 --- /dev/null +++ b/tasks/coverage/src/tools/esbuild.rs @@ -0,0 +1,85 @@ +use std::path::{Path, PathBuf}; +use std::process::Command; + +use oxc::span::SourceType; +use similar_asserts::SimpleDiff; + +use crate::workspace_root; +use crate::{ + suite::{Case, TestResult}, + test262::{Test262Case, TestFlag}, + Driver, +}; + +pub struct EsbuildTest262Case { + base: Test262Case, + workspace_root: PathBuf, +} + +impl Case for EsbuildTest262Case { + fn new(path: PathBuf, code: String) -> Self { + Self { base: Test262Case::new(path, code), workspace_root: workspace_root() } + } + + fn code(&self) -> &str { + self.base.code() + } + + fn path(&self) -> &Path { + self.base.path() + } + + fn test_result(&self) -> &TestResult { + self.base.test_result() + } + + fn skip_test_case(&self) -> bool { + self.base.should_fail() || self.base.skip_test_case() + } + + fn run(&mut self) { + let source_text = self.base.code(); + let is_module = self.base.meta().flags.contains(&TestFlag::Module); + let source_type = SourceType::default().with_module(is_module); + + let mut driver = + Driver { compress: true, codegen: true, remove_whitespace: true, ..Driver::default() }; + driver.run(source_text, source_type); + let oxc = driver.printed; + + if oxc.is_empty() { + self.base.set_result(TestResult::Passed); + return; + } + + let path = self.workspace_root.join(self.path()); + let esbuild = Command::new("esbuild") + .arg("--minify-whitespace=true") + .arg("--minify-syntax=true") + .arg("--keep-names") + .arg(&path) + .output() + .unwrap(); + + if !esbuild.stderr.is_empty() { + self.base.set_result(TestResult::Passed); + return; + } + let esbuild = String::from_utf8(esbuild.stdout).unwrap(); + + let esbuild_len = esbuild.len(); + let oxc_len = oxc.len(); + + if esbuild_len >= oxc_len { + self.base.set_result(TestResult::Passed); + return; + } + + let diff = oxc_len - esbuild_len; + + println!("\n{} {diff}\n", self.base.path().to_string_lossy()); + println!("\n{}\n", SimpleDiff::from_str(&oxc, &esbuild, "oxc", "esbuild")); + + self.base.set_result(TestResult::GenericError(">> ", format!("{diff}"))); + } +} diff --git a/tasks/minsize/minsize.snap b/tasks/minsize/minsize.snap index 565797d7fafe8..9c56cdb30e8f0 100644 --- a/tasks/minsize/minsize.snap +++ b/tasks/minsize/minsize.snap @@ -3,25 +3,25 @@ Original | minified | minified | gzip | gzip | Fixture ------------------------------------------------------------------------------------- 72.14 kB | 23.70 kB | 23.70 kB | 8.60 kB | 8.54 kB | react.development.js -173.90 kB | 59.79 kB | 59.82 kB | 19.41 kB | 19.33 kB | moment.js +173.90 kB | 59.68 kB | 59.82 kB | 19.40 kB | 19.33 kB | moment.js -287.63 kB | 90.08 kB | 90.07 kB | 32.03 kB | 31.95 kB | jquery.js +287.63 kB | 90.06 kB | 90.07 kB | 32.03 kB | 31.95 kB | jquery.js -342.15 kB | 118.11 kB | 118.14 kB | 44.44 kB | 44.37 kB | vue.js +342.15 kB | 118.07 kB | 118.14 kB | 44.44 kB | 44.37 kB | vue.js 544.10 kB | 71.76 kB | 72.48 kB | 26.15 kB | 26.20 kB | lodash.js -555.77 kB | 273.15 kB | 270.13 kB | 90.92 kB | 90.80 kB | d3.js +555.77 kB | 273.14 kB | 270.13 kB | 90.91 kB | 90.80 kB | d3.js -1.01 MB | 460.17 kB | 458.89 kB | 126.76 kB | 126.71 kB | bundle.min.js +1.01 MB | 459.65 kB | 458.89 kB | 126.75 kB | 126.71 kB | bundle.min.js -1.25 MB | 652.84 kB | 646.76 kB | 163.54 kB | 163.73 kB | three.js +1.25 MB | 652.73 kB | 646.76 kB | 163.55 kB | 163.73 kB | three.js -2.14 MB | 725.68 kB | 724.14 kB | 180.07 kB | 181.07 kB | victory.js +2.14 MB | 725.66 kB | 724.14 kB | 180.08 kB | 181.07 kB | victory.js -3.20 MB | 1.01 MB | 1.01 MB | 331.79 kB | 331.56 kB | echarts.js +3.20 MB | 1.01 MB | 1.01 MB | 331.78 kB | 331.56 kB | echarts.js -6.69 MB | 2.32 MB | 2.31 MB | 492.64 kB | 488.28 kB | antd.js +6.69 MB | 2.32 MB | 2.31 MB | 492.61 kB | 488.28 kB | antd.js -10.95 MB | 3.49 MB | 3.49 MB | 907.42 kB | 915.50 kB | typescript.js +10.95 MB | 3.49 MB | 3.49 MB | 907.29 kB | 915.50 kB | typescript.js