Skip to content

Commit

Permalink
feat(linter): remove the --timings feature (#2049)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Boshen authored Jan 16, 2024
1 parent f413297 commit 9e06bd7
Show file tree
Hide file tree
Showing 5 changed files with 12 additions and 120 deletions.
11 changes: 0 additions & 11 deletions crates/oxc_cli/src/command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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 .");
Expand Down
12 changes: 1 addition & 11 deletions crates/oxc_cli/src/lint/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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 = &[];
Expand Down
40 changes: 4 additions & 36 deletions crates/oxc_linter/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<Message<'a>> {
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);
}
}

Expand Down Expand Up @@ -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::<Vec<_>>();

timings.sort_by_key(|x| x.1);
let total = timings.iter().map(|x| x.1).sum::<Duration>().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)]
Expand Down
27 changes: 0 additions & 27 deletions crates/oxc_linter/src/rule_timer.rs

This file was deleted.

42 changes: 7 additions & 35 deletions crates/oxc_macros/src/declare_all_lint_rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down Expand Up @@ -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()
}
}

Expand Down Expand Up @@ -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<HashMap<&'static str, RuleTimer>> = Lazy::new(|| {
let mut m = HashMap::new();
#(m.insert(#struct_names::NAME, RuleTimer::new());)*
m
});

lazy_static::lazy_static! {
pub static ref RULES: Vec<RuleEnum> = vec![
#(RuleEnum::#struct_names(#struct_names::default())),*
Expand Down

0 comments on commit 9e06bd7

Please sign in to comment.