From 9e06bd779749a582939d68f33dafac1f9a90307a Mon Sep 17 00:00:00 2001 From: Boshen Date: Tue, 16 Jan 2024 14:21:04 +0800 Subject: [PATCH] feat(linter): remove the `--timings` feature (#2049) For a various reasons: This features bloats the code size. We have many tools for profiling in Rust (as compared to ESLint where the feature came from), so a built-in feature is not really needed anymore. ESLint needed `--timings` because it needs to monitor plugins. We control all our code so we don't need this. --- crates/oxc_cli/src/command.rs | 11 ----- crates/oxc_cli/src/lint/mod.rs | 12 +----- crates/oxc_linter/src/lib.rs | 40 ++---------------- crates/oxc_linter/src/rule_timer.rs | 27 ------------ .../src/declare_all_lint_rules/mod.rs | 42 ++++--------------- 5 files changed, 12 insertions(+), 120 deletions(-) delete mode 100644 crates/oxc_linter/src/rule_timer.rs diff --git a/crates/oxc_cli/src/command.rs b/crates/oxc_cli/src/command.rs index 6df646fe96d23..3ca052cdfc0da 100644 --- a/crates/oxc_cli/src/command.rs +++ b/crates/oxc_cli/src/command.rs @@ -66,10 +66,6 @@ impl FormatCommand { /// Miscellaneous #[derive(Debug, Clone, Bpaf)] pub struct MiscOptions { - /// Display the execution time of each lint rule - #[bpaf(switch, env("TIMING"), hide_usage)] - pub timing: bool, - /// list all the rules that are currently registered #[bpaf(switch, hide_usage)] pub rules: bool, @@ -262,17 +258,10 @@ mod misc_options { #[test] fn default() { let options = get_misc_options("."); - assert!(!options.timing); assert!(!options.rules); assert!(options.threads.is_none()); } - #[test] - fn timing() { - let options = get_misc_options("--timing ."); - assert!(options.timing); - } - #[test] fn threads() { let options = get_misc_options("--threads 4 ."); diff --git a/crates/oxc_cli/src/lint/mod.rs b/crates/oxc_cli/src/lint/mod.rs index 66452b9f4d14a..dceaab58d7cd2 100644 --- a/crates/oxc_cli/src/lint/mod.rs +++ b/crates/oxc_cli/src/lint/mod.rs @@ -36,10 +36,10 @@ impl Runner for LintRunner { warning_options, ignore_options, fix_options, - misc_options, codeowner_options, enable_plugins, config, + .. } = self.options; let mut paths = paths; @@ -97,7 +97,6 @@ impl Runner for LintRunner { .with_filter(filter) .with_config_path(config) .with_fix(fix_options.fix) - .with_timing(misc_options.timing) .with_import_plugin(enable_plugins.import_plugin) .with_jest_plugin(enable_plugins.jest_plugin) .with_jsx_a11y_plugin(enable_plugins.jsx_a11y_plugin) @@ -132,8 +131,6 @@ impl Runner for LintRunner { }); diagnostic_service.run(); - lint_service.linter().print_execution_times_if_enable(); - CliRunResult::LintResult(LintResult { duration: now.elapsed(), number_of_rules: lint_service.linter().number_of_rules(), @@ -208,13 +205,6 @@ mod test { } } - #[test] - fn timing() { - let args = &["--timing", "fixtures"]; - // make sure this doesn't crash - test(args); - } - #[test] fn no_arg() { let args = &[]; diff --git a/crates/oxc_linter/src/lib.rs b/crates/oxc_linter/src/lib.rs index 284f6334fe094..f7a7fc926e899 100644 --- a/crates/oxc_linter/src/lib.rs +++ b/crates/oxc_linter/src/lib.rs @@ -13,12 +13,11 @@ mod globals; mod options; pub mod partial_loader; pub mod rule; -mod rule_timer; mod rules; mod service; mod utils; -use std::{io::Write, rc::Rc, time::Duration}; +use std::{io::Write, rc::Rc}; use oxc_diagnostics::Report; pub(crate) use oxc_semantic::AstNode; @@ -131,33 +130,26 @@ impl Linter { self } - #[must_use] - pub fn with_print_execution_times(mut self, yes: bool) -> Self { - self.options.timing = yes; - self - } - pub fn run<'a>(&self, ctx: LintContext<'a>) -> Vec> { - let timing = self.options.timing; let semantic = Rc::clone(ctx.semantic()); let mut ctx = ctx.with_fix(self.options.fix); for (rule_name, rule) in &self.rules { ctx.with_rule_name(rule_name); - rule.run_once(&ctx, timing); + rule.run_once(&ctx); } for symbol in semantic.symbols().iter() { for (rule_name, rule) in &self.rules { ctx.with_rule_name(rule_name); - rule.run_on_symbol(symbol, &ctx, timing); + rule.run_on_symbol(symbol, &ctx); } } for node in semantic.nodes().iter() { for (rule_name, rule) in &self.rules { ctx.with_rule_name(rule_name); - rule.run(node, &ctx, timing); + rule.run(node, &ctx); } } @@ -187,30 +179,6 @@ impl Linter { } writeln!(writer, "Total: {}", RULES.len()).unwrap(); } - - #[allow(clippy::print_stdout)] - pub fn print_execution_times_if_enable(&self) { - if !self.options.timing { - return; - } - let mut timings = self - .rules - .iter() - .map(|(rule_name, rule)| (rule_name, rule.execute_time())) - .collect::>(); - - timings.sort_by_key(|x| x.1); - let total = timings.iter().map(|x| x.1).sum::().as_secs_f64(); - - println!("Rule timings in milliseconds:"); - println!("Total: {:.2}ms", total * 1000.0); - println!("{:>7} | {:>5} | Rule", "Time", "%"); - for (name, duration) in timings.iter().rev() { - let millis = duration.as_secs_f64() * 1000.0; - let relative = duration.as_secs_f64() / total * 100.0; - println!("{millis:>7.2} | {relative:>4.1}% | {name}"); - } - } } #[cfg(test)] diff --git a/crates/oxc_linter/src/rule_timer.rs b/crates/oxc_linter/src/rule_timer.rs deleted file mode 100644 index 56ff817e51ef9..0000000000000 --- a/crates/oxc_linter/src/rule_timer.rs +++ /dev/null @@ -1,27 +0,0 @@ -use std::{ - sync::atomic::{AtomicU32, AtomicU64, Ordering}, - time::Duration, -}; - -#[derive(Debug)] -pub struct RuleTimer { - pub secs: AtomicU64, - pub nanos: AtomicU32, -} - -impl RuleTimer { - pub const fn new() -> Self { - Self { secs: AtomicU64::new(0), nanos: AtomicU32::new(0) } - } - - pub fn update(&self, duration: &Duration) { - self.secs.fetch_add(duration.as_secs(), Ordering::SeqCst); - self.nanos.fetch_add(duration.subsec_nanos(), Ordering::SeqCst); - } - - pub fn duration(&self) -> Duration { - let secs = self.secs.load(Ordering::SeqCst); - let nanos = self.nanos.load(Ordering::SeqCst); - Duration::new(secs, nanos) - } -} diff --git a/crates/oxc_macros/src/declare_all_lint_rules/mod.rs b/crates/oxc_macros/src/declare_all_lint_rules/mod.rs index d86b8d8839f09..b4fb5e0831140 100644 --- a/crates/oxc_macros/src/declare_all_lint_rules/mod.rs +++ b/crates/oxc_macros/src/declare_all_lint_rules/mod.rs @@ -66,8 +66,7 @@ pub fn declare_all_lint_rules(metadata: AllLintRulesMeta) -> TokenStream { quote! { #(#use_stmts)* - use std::time::{Instant, Duration}; - use crate::{context::LintContext, rule::{Rule, RuleCategory, RuleMeta}, rule_timer:: RuleTimer, AstNode}; + use crate::{context::LintContext, rule::{Rule, RuleCategory, RuleMeta}, AstNode}; use oxc_semantic::SymbolId; #[derive(Debug, Clone)] @@ -109,41 +108,22 @@ pub fn declare_all_lint_rules(metadata: AllLintRulesMeta) -> TokenStream { } } - pub fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>, print_execution_times: bool) { - let start = print_execution_times.then(|| Instant::now()); - let result = match self { + pub fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { + match self { #(Self::#struct_names(rule) => rule.run(node, ctx)),* - }; - if let Some(start) = start { - RULE_TIMERS.get(self.name()).unwrap().update(&start.elapsed()); } - result } - pub fn run_on_symbol<'a>(&self, symbol_id: SymbolId, ctx: &LintContext<'a>, print_execution_times: bool) { - let start = print_execution_times.then(|| Instant::now()); - let result = match self { + pub fn run_on_symbol<'a>(&self, symbol_id: SymbolId, ctx: &LintContext<'a>) { + match self { #(Self::#struct_names(rule) => rule.run_on_symbol(symbol_id, ctx)),* - }; - if let Some(start) = start { - RULE_TIMERS.get(self.name()).unwrap().update(&start.elapsed()); } - result } - pub fn run_once<'a>(&self, ctx: &LintContext<'a>, print_execution_times: bool) { - let start = print_execution_times.then(|| Instant::now()); - let result = match self { + pub fn run_once<'a>(&self, ctx: &LintContext<'a>) { + match self { #(Self::#struct_names(rule) => rule.run_once(ctx)),* - }; - if let Some(start) = start { - RULE_TIMERS.get(self.name()).unwrap().update(&start.elapsed()); } - result - } - - pub fn execute_time(&self) -> Duration { - RULE_TIMERS.get(self.name()).unwrap().duration() } } @@ -173,14 +153,6 @@ pub fn declare_all_lint_rules(metadata: AllLintRulesMeta) -> TokenStream { } } - use once_cell::sync::Lazy; - use std::collections::HashMap; - pub static RULE_TIMERS: Lazy> = Lazy::new(|| { - let mut m = HashMap::new(); - #(m.insert(#struct_names::NAME, RuleTimer::new());)* - m - }); - lazy_static::lazy_static! { pub static ref RULES: Vec = vec![ #(RuleEnum::#struct_names(#struct_names::default())),*