Benchmark all rules (#3570)

astral-sh · Mar 17, 2023 · 87fab4a · 87fab4a
1 parent 2e21920
commit 87fab4a
Show file tree

Hide file tree

Showing 4 changed files with 67 additions and 15 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/crates/ruff_benchmark/Cargo.toml b/crates/ruff_benchmark/Cargo.toml
@@ -17,7 +17,10 @@ name = "linter"
 harness = false
 
 [dependencies]
+once_cell.workspace = true
 ruff.path = "../ruff"
+serde.workspace = true
+serde_json.workspace = true
 url = "2.3.1"
 ureq = "2.6.2"
 

diff --git a/crates/ruff_benchmark/benches/linter.rs b/crates/ruff_benchmark/benches/linter.rs
@@ -1,6 +1,10 @@
-use criterion::{black_box, criterion_group, criterion_main, BenchmarkId, Criterion, Throughput};
+use criterion::measurement::WallTime;
+use criterion::{
+    criterion_group, criterion_main, BenchmarkGroup, BenchmarkId, Criterion, Throughput,
+};
 use ruff::linter::lint_only;
 use ruff::settings::{flags, Settings};
+use ruff::RuleSelector;
 use ruff_benchmark::{TestCase, TestCaseSpeed, TestFile, TestFileDownloadError};
 use std::time::Duration;
 
@@ -22,43 +26,46 @@ static GLOBAL: tikv_jemallocator::Jemalloc = tikv_jemallocator::Jemalloc;
 
 fn create_test_cases() -> Result<Vec<TestCase>, TestFileDownloadError> {
     Ok(vec![
-        TestCase::fast(TestFile::try_download("numpy/globals.py", "https://github.com/numpy/numpy/blob/89d64415e349ca75a25250f22b874aa16e5c0973/numpy/_globals.py")?),
+        TestCase::fast(TestFile::try_download("numpy/globals.py", "https://raw.githubusercontent.com/numpy/numpy/89d64415e349ca75a25250f22b874aa16e5c0973/numpy/_globals.py")?),
         TestCase::normal(TestFile::try_download(
             "pydantic/types.py",
-            "https://raw.githubusercontent.com/pydantic/pydantic/main/pydantic/types.py",
+            "https://raw.githubusercontent.com/pydantic/pydantic/83b3c49e99ceb4599d9286a3d793cea44ac36d4b/pydantic/types.py",
         )?),
-        TestCase::normal(TestFile::try_download("numpy/ctypeslib.py", "https://github.com/numpy/numpy/blob/main/numpy/ctypeslib.py")?),
+        TestCase::normal(TestFile::try_download("numpy/ctypeslib.py", "https://raw.githubusercontent.com/numpy/numpy/e42c9503a14d66adfd41356ef5640c6975c45218/numpy/ctypeslib.py")?),
         TestCase::slow(TestFile::try_download(
             "large/dataset.py",
             "https://raw.githubusercontent.com/DHI/mikeio/b7d26418f4db2909b0aa965253dbe83194d7bb5b/tests/test_dataset.py",
         )?),
     ])
 }
 
-fn benchmark_linter(criterion: &mut Criterion) {
+fn benchmark_linter(mut group: BenchmarkGroup<WallTime>, settings: &Settings) {
     let test_cases = create_test_cases().unwrap();
-    let mut group = criterion.benchmark_group("linter");
 
     for case in test_cases {
         group.throughput(Throughput::Bytes(case.code().len() as u64));
         group.measurement_time(match case.speed() {
             TestCaseSpeed::Fast => Duration::from_secs(10),
             TestCaseSpeed::Normal => Duration::from_secs(20),
-            TestCaseSpeed::Slow => Duration::from_secs(30),
+            TestCaseSpeed::Slow => Duration::from_secs(45),
         });
+
         group.bench_with_input(
             BenchmarkId::from_parameter(case.name()),
             &case,
             |b, case| {
                 b.iter(|| {
-                    lint_only(
+                    let result = lint_only(
                         case.code(),
                         &case.path(),
                         None,
-                        &black_box(Settings::default()),
+                        settings,
                         flags::Noqa::Enabled,
                         flags::Autofix::Enabled,
-                    )
+                    );
+
+                    // Assert that file contains no parse errors
+                    assert_eq!(result.error, None);
                 });
             },
         );
@@ -67,5 +74,21 @@ fn benchmark_linter(criterion: &mut Criterion) {
     group.finish();
 }
 
-criterion_group!(benches, benchmark_linter);
-criterion_main!(benches);
+fn benchmark_default_rules(criterion: &mut Criterion) {
+    let group = criterion.benchmark_group("linter/default-rules");
+    benchmark_linter(group, &Settings::default());
+}
+
+fn benchmark_all_rules(criterion: &mut Criterion) {
+    let settings = Settings {
+        rules: RuleSelector::All.into_iter().into(),
+        ..Settings::default()
+    };
+
+    let group = criterion.benchmark_group("linter/all-rules");
+    benchmark_linter(group, &settings);
+}
+
+criterion_group!(default_rules, benchmark_default_rules);
+criterion_group!(all_rules, benchmark_all_rules);
+criterion_main!(default_rules, all_rules);
diff --git a/crates/ruff_benchmark/src/lib.rs b/crates/ruff_benchmark/src/lib.rs
@@ -1,5 +1,6 @@
 use std::fmt::{Display, Formatter};
-use std::path::{Path, PathBuf};
+use std::path::PathBuf;
+use std::process::Command;
 use url::Url;
 
 /// Relative size of a test case. Benchmarks can use it to configure the time for how long a benchmark should run to get stable results.
@@ -58,7 +59,7 @@ impl TestCase {
     }
 
     pub fn path(&self) -> PathBuf {
-        Path::new("target").join(self.name())
+        TARGET_DIR.join(self.name())
     }
 }
 
@@ -68,6 +69,28 @@ pub struct TestFile {
     code: String,
 }
 
+static TARGET_DIR: once_cell::sync::Lazy<PathBuf> = once_cell::sync::Lazy::new(|| {
+    cargo_target_directory().unwrap_or_else(|| PathBuf::from("target"))
+});
+
+fn cargo_target_directory() -> Option<PathBuf> {
+    #[derive(serde::Deserialize)]
+    struct Metadata {
+        target_directory: PathBuf,
+    }
+
+    std::env::var_os("CARGO_TARGET_DIR")
+        .map(PathBuf::from)
+        .or_else(|| {
+            let output = Command::new(std::env::var_os("CARGO")?)
+                .args(["metadata", "--format-version", "1"])
+                .output()
+                .ok()?;
+            let metadata: Metadata = serde_json::from_slice(&output.stdout).ok()?;
+            Some(metadata.target_directory)
+        })
+}
+
 impl TestFile {
     pub fn new(name: String, code: String) -> Self {
         Self { name, code }
@@ -77,7 +100,7 @@ impl TestFile {
     pub fn try_download(name: &str, url: &str) -> Result<TestFile, TestFileDownloadError> {
         let url = Url::parse(url)?;
 
-        let cached_filename = Path::new("target").join(name);
+        let cached_filename = TARGET_DIR.join(name);
 
         if let Ok(content) = std::fs::read_to_string(&cached_filename) {
             Ok(TestFile::new(name.to_string(), content))