diff --git a/.cargo/config.toml b/.cargo/config.toml new file mode 100644 index 0000000..8aa067b --- /dev/null +++ b/.cargo/config.toml @@ -0,0 +1,14 @@ +# Build acceleration via sccache. +# +# `rustc-wrapper` routes every rustc invocation through sccache's shared compile +# cache, so clean builds and branch switches reuse previously-compiled artifacts +# (the pinned oxc / ruff git dependencies dominate build time and cache well). +# +# Requires `sccache` on PATH: +# cargo install sccache --locked # or: brew install sccache +# +# CI provisions it via mozilla-actions/sccache-action. If you do not have +# sccache installed, either install it (one command above) or override locally +# with `CARGO_BUILD_RUSTC_WRAPPER=""` / `RUSTC_WRAPPER=""`. +[build] +rustc-wrapper = "sccache" diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 21343d4..f750c12 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -17,6 +17,12 @@ permissions: env: CARGO_TERM_COLOR: always RUST_BACKTRACE: 1 + # sccache: .cargo/config.toml sets rustc-wrapper = "sccache", so every + # compiling job must have sccache on PATH. The action below installs it and + # these vars point it at the GitHub Actions cache backend. Incremental + # compilation is disabled because sccache cannot cache incremental units. + SCCACHE_GHA_ENABLED: "true" + CARGO_INCREMENTAL: 0 jobs: fmt: @@ -29,6 +35,7 @@ jobs: with: components: rustfmt - uses: Swatinem/rust-cache@v2 + - uses: mozilla-actions/sccache-action@v0.0.9 - run: cargo fmt --all --check clippy: @@ -45,6 +52,7 @@ jobs: with: components: clippy - uses: Swatinem/rust-cache@v2 + - uses: mozilla-actions/sccache-action@v0.0.9 - run: cargo clippy --workspace --exclude conformance --all-targets -- -D warnings test: @@ -59,6 +67,7 @@ jobs: - uses: actions/checkout@v4 - uses: dtolnay/rust-toolchain@stable - uses: Swatinem/rust-cache@v2 + - uses: mozilla-actions/sccache-action@v0.0.9 - run: cargo test --workspace --exclude conformance deny: diff --git a/.github/workflows/publish.yaml b/.github/workflows/publish.yaml index e05d92b..126d811 100644 --- a/.github/workflows/publish.yaml +++ b/.github/workflows/publish.yaml @@ -184,6 +184,12 @@ jobs: needs: [meta, create_release] if: needs.meta.outputs.release_assets_exist != 'true' runs-on: ${{ matrix.os }} + # .cargo/config.toml sets rustc-wrapper = "sccache"; native builds get + # sccache from the action below. The Alpine container build neutralizes the + # wrapper instead (see that step), since rust:alpine has no sccache. + env: + SCCACHE_GHA_ENABLED: "true" + CARGO_INCREMENTAL: 0 strategy: fail-fast: false matrix: @@ -230,6 +236,10 @@ jobs: with: key: ${{ matrix.triple }} + - name: Setup sccache + if: ${{ !matrix.alpine }} + uses: mozilla-actions/sccache-action@v0.0.9 + # ruby-prism-sys runs bindgen at build time and needs libclang + libc # headers; native Linux runners get them from apt. - name: Install clang for bindgen (Linux) @@ -256,9 +266,13 @@ jobs: # dynamically linked (-crt-static off) to dlopen libclang. Building # host-native applies the flag everywhere. Then place the binary at the # triple path the shared packaging step expects. + # RUSTC_WRAPPER="" disables the sccache wrapper that .cargo/config.toml + # sets repo-wide; rust:alpine has no sccache, and the env var overrides + # the config value. docker run --rm -v "$PWD":/work -w /work \ -e CARGO_TARGET_DIR=/work/target \ -e LIBCLANG_PATH=/usr/lib \ + -e RUSTC_WRAPPER="" \ -e RUSTFLAGS="-C target-feature=-crt-static" \ rust:alpine sh -c ' set -e diff --git a/Cargo.lock b/Cargo.lock index b214d85..78c504a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -101,6 +101,15 @@ dependencies = [ "rowan 0.12.6", ] +[[package]] +name = "aligned-vec" +version = "0.6.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "dc890384c8602f339876ded803c97ad529f3842aba97f6392b3dba0dd171769b" +dependencies = [ + "equator", +] + [[package]] name = "alloca" version = "0.4.0" @@ -674,6 +683,12 @@ version = "3.20.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "72f5acc6cb2ba439de613abc23857ec3d78374d8ed5ac84e9d11336e87da8649" +[[package]] +name = "bytemuck" +version = "1.25.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c8efb64bd706a16a1bdde310ae86b351e4d21550d98d056f22f8a7f7a2183fec" + [[package]] name = "byteorder" version = "1.5.0" @@ -1055,6 +1070,15 @@ version = "0.1.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "417bef24afe1460300965a25ff4a24b8b45ad011948302ec221e8a0a81eb2c79" +[[package]] +name = "cpp_demangle" +version = "0.4.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f2bb79cb74d735044c972aae58ed0aaa9a837e85b01106a54c39e42e97f62253" +dependencies = [ + "cfg-if", +] + [[package]] name = "cpufeatures" version = "0.2.17" @@ -1230,6 +1254,15 @@ dependencies = [ "parking_lot_core", ] +[[package]] +name = "debugid" +version = "0.8.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bef552e6f588e446098f6ba40d89ac146c8c7b64aade83c051ee00bb5d2bc18d" +dependencies = [ + "uuid", +] + [[package]] name = "defmt" version = "1.1.0" @@ -1462,6 +1495,26 @@ dependencies = [ "syn 2.0.118", ] +[[package]] +name = "equator" +version = "0.4.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4711b213838dfee0117e3be6ac926007d7f433d7bbe33595975d4190cb07e6fc" +dependencies = [ + "equator-macro", +] + +[[package]] +name = "equator-macro" +version = "0.4.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "44f23cf4b44bfce11a86ace86f8a73ffdec849c9fd00a386a53d278bd9e81fb3" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.118", +] + [[package]] name = "equivalent" version = "1.0.2" @@ -1550,6 +1603,18 @@ version = "0.1.9" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5baebc0774151f905a1a2cc41989300b1e6fbb29aff0ceffa1064fdd3088d582" +[[package]] +name = "findshlibs" +version = "0.10.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "40b9e59cd0f7e0806cca4be089683ecb6434e602038df21fe6bf6711b2f07f64" +dependencies = [ + "cc", + "lazy_static", + "libc", + "winapi", +] + [[package]] name = "fixedbitset" version = "0.5.7" @@ -2412,6 +2477,24 @@ dependencies = [ "rustversion", ] +[[package]] +name = "inferno" +version = "0.11.21" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "232929e1d75fe899576a3d5c7416ad0d88dbfbb3c3d6aa00873a7408a50ddb88" +dependencies = [ + "ahash", + "indexmap", + "is-terminal", + "itoa", + "log", + "num-format", + "once_cell", + "quick-xml 0.26.0", + "rgb", + "str_stack", +] + [[package]] name = "inotify" version = "0.11.2" @@ -3191,6 +3274,15 @@ version = "2.8.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "88904434abc2901f197fe8cc55f0445e7ded921dba5911dad2e2b39b48e663c4" +[[package]] +name = "memmap2" +version = "0.9.11" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d1219ed1b7f229ee7104d281dd01d6802fe28bb6e95d292942c4daacdeb798c0" +dependencies = [ + "libc", +] + [[package]] name = "memoffset" version = "0.6.5" @@ -3261,6 +3353,17 @@ dependencies = [ "uuid", ] +[[package]] +name = "nix" +version = "0.26.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "598beaf3cc6fdd9a5dfb1630c2800c7acd31df7aaf0f565796fba2b53ca1af1b" +dependencies = [ + "bitflags 1.3.2", + "cfg-if", + "libc", +] + [[package]] name = "nodejs-built-in-modules" version = "1.0.0" @@ -3347,6 +3450,16 @@ version = "0.2.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "521739c6d2bac4aa25192232afe6841231376b2b26d4d9fae5ecf8ca5772e441" +[[package]] +name = "num-format" +version = "0.4.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a652d9771a63711fd3c3deb670acfbe5c30a4072e664d7a3bf5a9e1056ac72c3" +dependencies = [ + "arrayvec", + "itoa", +] + [[package]] name = "num-integer" version = "0.1.46" @@ -4463,6 +4576,7 @@ dependencies = [ "poly-cache", "poly-catalog", "poly-config", + "pprof", "pretty_graphql", "pretty_yaml", "rayon", @@ -4526,6 +4640,28 @@ version = "0.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "439ee305def115ba05938db6eb1644ff94165c5ab5e9420d1c1bcedbba909391" +[[package]] +name = "pprof" +version = "0.15.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "38a01da47675efa7673b032bf8efd8214f1917d89685e07e395ab125ea42b187" +dependencies = [ + "aligned-vec", + "backtrace", + "cfg-if", + "findshlibs", + "inferno", + "libc", + "log", + "nix", + "once_cell", + "smallvec", + "spin", + "symbolic-demangle", + "tempfile", + "thiserror 2.0.18", +] + [[package]] name = "ppv-lite86" version = "0.2.21" @@ -4698,12 +4834,21 @@ dependencies = [ "chrono", "indexmap", "newtype-uuid", - "quick-xml", + "quick-xml 0.38.4", "strip-ansi-escapes", "thiserror 2.0.18", "uuid", ] +[[package]] +name = "quick-xml" +version = "0.26.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7f50b1c63b38611e7d4d7f68b82d3ad0cc71a2ad2e7f61fc10f1328d917c93cd" +dependencies = [ + "memchr", +] + [[package]] name = "quick-xml" version = "0.38.4" @@ -4918,6 +5063,15 @@ version = "0.8.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d6f6ff9a378485b298a5286656da665ba74413d36db0979633275d2e708145d4" +[[package]] +name = "rgb" +version = "0.8.53" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "47b34b781b31e5d73e9fbc8689c70551fd1ade9a19e3e28cfec8580a79290cc4" +dependencies = [ + "bytemuck", +] + [[package]] name = "ring" version = "0.17.14" @@ -6059,6 +6213,15 @@ dependencies = [ "winapi", ] +[[package]] +name = "spin" +version = "0.10.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d5fe4ccb98d9c292d56fec89a5e07da7fc4cf0dc11e156b41793132775d3e591" +dependencies = [ + "lock_api", +] + [[package]] name = "sqruff-lib" version = "0.38.0" @@ -6135,6 +6298,12 @@ version = "1.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a2eb9349b6444b326872e140eb1cf5e7c522154d69e7a0ffb0fb81c06b37543f" +[[package]] +name = "str_stack" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7f446288b699d66d0fd2e30d1cfe7869194312524b3b9252594868ed26ef056a" + [[package]] name = "streaming-iterator" version = "0.1.9" @@ -6208,6 +6377,29 @@ version = "3.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e396b6523b11ccb83120b115a0b7366de372751aa6edf19844dfb13a6af97e91" +[[package]] +name = "symbolic-common" +version = "12.18.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "332615d90111d8eeaf86a84dc9bbe9f65d0d8c5cf11b4caccedc37754eb0dcfd" +dependencies = [ + "debugid", + "memmap2", + "stable_deref_trait", + "uuid", +] + +[[package]] +name = "symbolic-demangle" +version = "12.18.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "912017718eb4d21930546245af9a3475c9dccf15675a5c215664e76621afc471" +dependencies = [ + "cpp_demangle", + "rustc-demangle", + "symbolic-common", +] + [[package]] name = "syn" version = "1.0.109" diff --git a/crates/poly-cache/src/lib.rs b/crates/poly-cache/src/lib.rs index 06da932..34393d8 100644 --- a/crates/poly-cache/src/lib.rs +++ b/crates/poly-cache/src/lib.rs @@ -80,8 +80,10 @@ //! are implemented, add `fd-lock` or `fs2` to the workspace and open `.lock` with //! an exclusive `FileLock` before mutating the directory tree. +use std::collections::HashSet; use std::path::{Path, PathBuf}; -use std::sync::atomic::{AtomicU64, Ordering}; +use std::sync::atomic::{AtomicBool, AtomicU64, Ordering}; +use std::sync::{Arc, Mutex}; mod maintenance; @@ -177,6 +179,118 @@ impl Namespace { Namespace::Hook => "hook", } } + + /// Dense index into the per-namespace [`PresenceIndex`] arrays, matching the + /// order of [`Namespace::ALL`]. + fn slot(self) -> usize { + match self { + Namespace::Lint => 0, + Namespace::Fmt => 1, + Namespace::Hook => 2, + } + } +} + +// --------------------------------------------------------------------------- +// PresenceIndex +// --------------------------------------------------------------------------- + +/// An in-memory snapshot of which cache keys exist on disk, taken once when the +/// cache is opened. +/// +/// # Why +/// +/// A whole-repository run issues one [`ResultCache::get`] per (file, engine). On +/// a cold or sparse cache the overwhelming majority of those are misses, and +/// before this index every miss cost an `open(2)` that returned `ENOENT` — tens +/// of thousands of failed syscalls that made a cold run slower than running the +/// tools with no cache at all. The index turns a miss into an in-memory hash +/// lookup; only a confirmed hit touches the filesystem. +/// +/// # Snapshot + session layers +/// +/// `snapshot` is built once at open and never mutated, so it is read lock-free +/// across rayon workers — this is the layer that turns the millions of cold +/// misses into hash lookups. `added` records keys written by [`ResultCache::put`] +/// during the current process so that a get of a just-put key still hits (the +/// runner never does this, but the library contract and direct callers expect +/// it). The `any_added` flag keeps the hot path lock-free: until the first put, +/// every miss skips the `added` mutex entirely, and because puts are +/// duration-gated to expensive results, most runs never set it. +#[derive(Debug)] +struct PresenceIndex { + /// Immutable on-disk key set per namespace, indexed by [`Namespace::slot`]. + snapshot: [HashSet; 3], + /// Keys written during this process, per namespace; consulted only when + /// [`any_added`](Self::any_added) is set. + added: [Mutex>; 3], + /// Whether any [`put`](ResultCache::put) has run; lets `contains` skip the + /// `added` mutex on the common (no-write) path. + any_added: AtomicBool, +} + +impl PresenceIndex { + /// An empty index (no keys present) — used for a disabled cache. + fn empty() -> Self { + Self { + snapshot: [HashSet::new(), HashSet::new(), HashSet::new()], + added: [ + Mutex::new(HashSet::new()), + Mutex::new(HashSet::new()), + Mutex::new(HashSet::new()), + ], + any_added: AtomicBool::new(false), + } + } + + /// Scan each namespace directory once, recording the entry file names (the + /// hex cache keys). `.`-prefixed temporaries from in-flight [`put`]s are + /// skipped so a half-written entry is never treated as present. + /// + /// [`put`]: ResultCache::put + fn scan(root: &Path) -> Self { + let mut index = Self::empty(); + for namespace in Namespace::ALL { + let dir = root.join("results").join(namespace.as_dir()); + let Ok(read_dir) = std::fs::read_dir(&dir) else { + continue; + }; + let set = &mut index.snapshot[namespace.slot()]; + for entry in read_dir.flatten() { + if let Ok(name) = entry.file_name().into_string() + && !name.starts_with('.') + { + set.insert(name); + } + } + } + index + } + + /// Whether a key may exist on disk for `namespace` (snapshot or written this + /// process). + fn contains(&self, namespace: Namespace, key: &CacheKey) -> bool { + if self.snapshot[namespace.slot()].contains(key.as_str()) { + return true; + } + // Fast path: no put has happened, so the `added` set is empty. + if !self.any_added.load(Ordering::Acquire) { + return false; + } + self.added[namespace.slot()] + .lock() + .expect("presence index mutex poisoned") + .contains(key.as_str()) + } + + /// Record a key written by [`put`](ResultCache::put) so later gets hit. + fn record(&self, namespace: Namespace, key: &CacheKey) { + self.added[namespace.slot()] + .lock() + .expect("presence index mutex poisoned") + .insert(key.as_str().to_owned()); + self.any_added.store(true, Ordering::Release); + } } // --------------------------------------------------------------------------- @@ -268,6 +382,9 @@ pub struct ResultCache { /// `/.polylint/cache/` root: PathBuf, enabled: bool, + /// Snapshot of on-disk keys, taken at open; lets `get` skip a syscall on a + /// miss. Shared (lock-free) across clones and rayon workers. + present: Arc, } impl ResultCache { @@ -280,10 +397,19 @@ impl ResultCache { /// When `enabled`, creates the full sub-directory tree and writes the /// `VERSION` sentinel. When disabled, returns a no-op stub. pub fn open(root: PathBuf, enabled: bool) -> anyhow::Result { - if enabled { + let present = if enabled { Self::init_dirs(&root)?; - } - Ok(Self { root, enabled }) + // Scan after init_dirs so the namespace directories exist; a fresh + // tree simply yields empty sets. + PresenceIndex::scan(&root) + } else { + PresenceIndex::empty() + }; + Ok(Self { + root, + enabled, + present: Arc::new(present), + }) } /// Open the cache by walking upward from `start` to find the repo root. @@ -469,8 +595,12 @@ impl ResultCache { // ----------------------------------------------------------------------- /// Fetch a cached entry by key, or `None` on miss / when disabled. + /// + /// A key absent from the open-time [`PresenceIndex`] returns `None` without + /// touching the filesystem, so a miss costs a hash lookup rather than a + /// failed `open(2)`. pub fn get(&self, namespace: Namespace, key: &CacheKey) -> Option> { - if !self.enabled { + if !self.enabled || !self.present.contains(namespace, key) { return None; } std::fs::read(self.entry_path(namespace, key)).ok() @@ -499,6 +629,9 @@ impl ResultCache { .map_err(|e| anyhow::anyhow!("cache write {}: {e}", tmp.display()))?; std::fs::rename(&tmp, &dest) .map_err(|e| anyhow::anyhow!("cache rename to {}: {e}", dest.display()))?; + // Make the freshly-written entry visible to subsequent gets on this + // handle (the open-time snapshot predates it). + self.present.record(namespace, key); Ok(()) } @@ -557,6 +690,40 @@ mod tests { ); } + /// The presence index is an open-time snapshot, so an entry written by a + /// *previous* handle (a prior run) must be found by a freshly-opened cache. + #[test] + fn reopened_cache_sees_prior_puts_via_snapshot() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path().join("cache"); + let digest = ResultCache::single_file_digest("content"); + let key = ResultCache::key(Namespace::Lint, "eng", "1", &empty_args(), &digest); + { + let first = ResultCache::open(root.clone(), true).expect("open cache"); + first.put(Namespace::Lint, &key, b"stored").unwrap(); + } + // A new handle scans the on-disk tree at open and must see the entry. + let second = ResultCache::open(root, true).expect("reopen cache"); + assert_eq!( + second.get(Namespace::Lint, &key).as_deref(), + Some(&b"stored"[..]), + "reopened cache must find an entry a prior handle wrote" + ); + } + + /// A key never written must miss even after *other* keys have been put (the + /// `any_added` fast path and the on-disk file must agree). + #[test] + fn unwritten_key_misses_after_unrelated_put() { + let tmp = tempfile::tempdir().unwrap(); + let cache = cache_at(&tmp); + let digest = ResultCache::single_file_digest("content"); + let written = ResultCache::key(Namespace::Lint, "eng", "1", &empty_args(), &digest); + cache.put(Namespace::Lint, &written, b"stored").unwrap(); + let other = ResultCache::key(Namespace::Lint, "other", "1", &empty_args(), &digest); + assert_eq!(cache.get(Namespace::Lint, &other), None); + } + #[test] fn miss_when_content_changes() { let tmp = tempfile::tempdir().unwrap(); diff --git a/crates/polylint-core/Cargo.toml b/crates/polylint-core/Cargo.toml index 552ddcc..8485a40 100644 --- a/crates/polylint-core/Cargo.toml +++ b/crates/polylint-core/Cargo.toml @@ -91,6 +91,9 @@ which.workspace = true criterion.workspace = true insta.workspace = true owo-colors.workspace = true +# pprof — CPU sampling for the bench flamegraph profiler (see +# benches/support/profiler.rs). `flamegraph` pulls in the inferno SVG writer. +pprof = { workspace = true, features = ["flamegraph"] } [[bench]] name = "generic_formatter" diff --git a/crates/polylint-core/benches/cache_key.rs b/crates/polylint-core/benches/cache_key.rs index d92e728..dc1e20a 100644 --- a/crates/polylint-core/benches/cache_key.rs +++ b/crates/polylint-core/benches/cache_key.rs @@ -84,5 +84,56 @@ fn bench_cache_key(c: &mut Criterion) { }); } -criterion_group!(benches, bench_cache_key); +/// Cache `get` on a miss vs a hit. The miss path is the one a whole-repo run +/// hits tens of thousands of times: it must be served from the in-memory +/// presence index with no syscall (it used to cost a failing `open()`), so a +/// regression that reintroduces a per-miss syscall shows up here as the miss +/// approaching the hit's cost. +fn bench_cache_get(c: &mut Criterion) { + let dir = tempfile::tempdir().expect("tempdir"); + let root = dir.path().join("cache"); + let args = ResultCache::serialize_args(&toml::Table::new()); + + // Populate one entry, then reopen so the presence-index snapshot includes it + // (mirrors a warm run that scans the on-disk tree at startup). + { + let cache = ResultCache::open(root.clone(), true).expect("open cache"); + let digest = ResultCache::single_file_digest("cached payload"); + let present = ResultCache::key_with_args(Namespace::Lint, "eng", "1", &args, &digest); + cache.put(Namespace::Lint, &present, b"payload").expect("put"); + } + let cache = ResultCache::open(root, true).expect("reopen cache"); + let present = ResultCache::key_with_args( + Namespace::Lint, + "eng", + "1", + &args, + &ResultCache::single_file_digest("cached payload"), + ); + let absent = ResultCache::key_with_args( + Namespace::Lint, + "eng", + "1", + &args, + &ResultCache::single_file_digest("never stored"), + ); + + let mut group = c.benchmark_group("cache_get"); + group.bench_function("miss_via_presence_index", |b| { + b.iter(|| black_box(cache.get(Namespace::Lint, black_box(&absent)))); + }); + group.bench_function("hit_reads_file", |b| { + b.iter(|| black_box(cache.get(Namespace::Lint, black_box(&present)))); + }); + group.finish(); +} + +#[path = "support/profiler.rs"] +mod profiler; + +criterion_group! { + name = benches; + config = Criterion::default().with_profiler(profiler::FlamegraphProfiler::new(997)); + targets = bench_cache_key, bench_cache_get +} criterion_main!(benches); diff --git a/crates/polylint-core/benches/generic_formatter.rs b/crates/polylint-core/benches/generic_formatter.rs index 28f488c..28be8eb 100644 --- a/crates/polylint-core/benches/generic_formatter.rs +++ b/crates/polylint-core/benches/generic_formatter.rs @@ -143,5 +143,12 @@ fn bench_generic_formatter(c: &mut Criterion) { group.finish(); } -criterion_group!(benches, bench_generic_formatter); +#[path = "support/profiler.rs"] +mod profiler; + +criterion_group! { + name = benches; + config = Criterion::default().with_profiler(profiler::FlamegraphProfiler::new(997)); + targets = bench_generic_formatter +} criterion_main!(benches); diff --git a/crates/polylint-core/benches/runner_e2e.rs b/crates/polylint-core/benches/runner_e2e.rs index 8fc6c5c..b365847 100644 --- a/crates/polylint-core/benches/runner_e2e.rs +++ b/crates/polylint-core/benches/runner_e2e.rs @@ -85,5 +85,12 @@ fn bench_runner(c: &mut Criterion) { group.finish(); } -criterion_group!(benches, bench_runner); +#[path = "support/profiler.rs"] +mod profiler; + +criterion_group! { + name = benches; + config = Criterion::default().with_profiler(profiler::FlamegraphProfiler::new(997)); + targets = bench_runner +} criterion_main!(benches); diff --git a/crates/polylint-core/benches/support/profiler.rs b/crates/polylint-core/benches/support/profiler.rs new file mode 100644 index 0000000..3d012d1 --- /dev/null +++ b/crates/polylint-core/benches/support/profiler.rs @@ -0,0 +1,85 @@ +//! A criterion [`Profiler`] that captures a CPU flamegraph for any bench run +//! under `--profile-time`. +//! +//! Criterion calls [`start_profiling`]/[`stop_profiling`] only when a bench is +//! invoked with `--profile-time=`; normal `cargo bench` runs are +//! unaffected. While profiling, [`pprof`] samples the call stack at a fixed +//! frequency and, on stop, writes a `flamegraph.svg` into criterion's +//! per-benchmark output directory (`target/criterion///profile/`). +//! +//! This wraps [`pprof::ProfilerGuard`] directly rather than enabling pprof's own +//! `criterion` feature, so it stays decoupled from whichever criterion version +//! that feature happens to pin. +//! +//! [`start_profiling`]: Profiler::start_profiling +//! [`stop_profiling`]: Profiler::stop_profiling + +use std::ffi::c_int; +use std::fs::File; +use std::path::Path; + +use criterion::profiler::Profiler; +use pprof::ProfilerGuard; + +/// Sampling-based flamegraph profiler for criterion `--profile-time` runs. +pub struct FlamegraphProfiler<'a> { + /// Sampling frequency in Hz. + frequency: c_int, + /// The active pprof guard while a bench is being profiled. + guard: Option>, +} + +impl FlamegraphProfiler<'_> { + /// Build a profiler sampling at `frequency` Hz. 997 (a prime near 1 kHz) is + /// a good default — high enough for detail, prime to avoid aliasing with + /// periodic work. + pub fn new(frequency: c_int) -> Self { + Self { + frequency, + guard: None, + } + } +} + +impl Profiler for FlamegraphProfiler<'_> { + fn start_profiling(&mut self, _benchmark_id: &str, _benchmark_dir: &Path) { + match ProfilerGuard::new(self.frequency) { + Ok(guard) => self.guard = Some(guard), + Err(error) => eprintln!("flamegraph profiler: failed to start pprof: {error}"), + } + } + + fn stop_profiling(&mut self, _benchmark_id: &str, benchmark_dir: &Path) { + let Some(guard) = self.guard.take() else { + return; + }; + if let Err(error) = std::fs::create_dir_all(benchmark_dir) { + eprintln!( + "flamegraph profiler: create {}: {error}", + benchmark_dir.display() + ); + return; + } + let svg_path = benchmark_dir.join("flamegraph.svg"); + let report = match guard.report().build() { + Ok(report) => report, + Err(error) => { + eprintln!("flamegraph profiler: build report: {error}"); + return; + } + }; + match File::create(&svg_path) { + Ok(file) => { + if let Err(error) = report.flamegraph(file) { + eprintln!("flamegraph profiler: write {}: {error}", svg_path.display()); + } else { + eprintln!("flamegraph profiler: wrote {}", svg_path.display()); + } + } + Err(error) => eprintln!( + "flamegraph profiler: create {}: {error}", + svg_path.display() + ), + } + } +} diff --git a/crates/polylint-core/src/runner.rs b/crates/polylint-core/src/runner.rs index 7b6501e..ca78727 100644 --- a/crates/polylint-core/src/runner.rs +++ b/crates/polylint-core/src/runner.rs @@ -79,6 +79,19 @@ pub struct FormatResult { /// others, so re-lint until stable, but cap to guarantee termination. const MAX_FIX_PASSES: usize = 5; +/// Minimum engine runtime for a result to be worth caching. +/// +/// Persisting a result costs a serialize + atomic temp-write + rename, and +/// reading it back costs an open + read + deserialize. When an engine produced a +/// result in less time than this, that round-trip is slower than just recomputing +/// it — and on a whole-repository run the overwhelming majority of files fall +/// here, so unconditionally caching them made a cold run several times slower than +/// `--no-cache` (one tiny cache file per file × engine). Only results that took at +/// least this long are persisted; cheaper ones are recomputed every run, which is +/// by construction fast. Genuinely expensive files (large/slow inputs) still get +/// cached and skip the work on the next run. +const MIN_CACHE_DURATION: std::time::Duration = std::time::Duration::from_millis(5); + /// One engine paired with its resolved config and once-serialised cache args. /// /// Built once per language (not per file) so the per-file rayon loop neither @@ -358,32 +371,43 @@ fn lint_content( language: f.language.clone(), content: Arc::from(content), }; - // Content is constant across this file's engines, so digest it once. - let digest = ResultCache::single_file_digest(content); + // The digest (and the per-engine keys derived from it) are only needed to + // address the cache, so skip them entirely when the cache is disabled. + let digest = cache + .enabled() + .then(|| ResultCache::single_file_digest(content)); let mut all = Vec::new(); let mut debug = collect_debug.then(RunDebug::default); let engine_plans = plans.get(&f.language).map(Vec::as_slice).unwrap_or(&[]); for plan in engine_plans { - let key = ResultCache::key_with_args( - Namespace::Lint, - plan.engine.name(), - plan.engine.version(), - &plan.serialized_args, - &digest, - ); - if let Some(bytes) = cache.get(Namespace::Lint, &key) + let key = digest.as_ref().map(|digest| { + ResultCache::key_with_args( + Namespace::Lint, + plan.engine.name(), + plan.engine.version(), + &plan.serialized_args, + digest, + ) + }); + if let Some(key) = &key + && let Some(bytes) = cache.get(Namespace::Lint, key) && let Ok(diags) = serde_json::from_slice::>(&bytes) { push_engine_debug(debug.as_mut(), plan, None); all.extend(diags); continue; } - // Time the engine only when debug collection is on (zero cost otherwise). - let started = collect_debug.then(std::time::Instant::now); + let started = std::time::Instant::now(); let diags = plan.engine.lint(&src, &plan.config)?; - push_engine_debug(debug.as_mut(), plan, started); - if let Ok(bytes) = serde_json::to_vec(&diags) - && let Err(error) = cache.put(Namespace::Lint, &key, &bytes) + let elapsed = started.elapsed(); + note_slow_engine(&f.path, content.len(), plan.engine.name(), elapsed); + push_engine_debug(debug.as_mut(), plan, Some(started)); + // Persist only results expensive enough that reloading beats recomputing + // (see MIN_CACHE_DURATION). + if let Some(key) = &key + && elapsed >= MIN_CACHE_DURATION + && let Ok(bytes) = serde_json::to_vec(&diags) + && let Err(error) = cache.put(Namespace::Lint, key, &bytes) { tracing::warn!( engine = plan.engine.name(), @@ -417,6 +441,64 @@ fn push_engine_debug( } } +/// A single engine taking at least this long on one file is surfaced at `warn`: +/// at this scale it is almost always a pathological input for that backend (a +/// huge generated file, or a backend with super-linear behaviour on certain +/// shapes), and it serialises a whole-repo run on one core. +const SLOW_ENGINE_WARN: std::time::Duration = std::time::Duration::from_secs(2); + +/// A run at least this long is surfaced at `info` (visible under `-v`), so the +/// per-file cost is observable before it reaches the `warn` threshold. +const SLOW_ENGINE_INFO: std::time::Duration = std::time::Duration::from_millis(250); + +/// The generic tree-sitter tier; slow runs here are our own code, not a wrapped +/// upstream tool, so the upstream-issue hint is omitted for it. +const GENERIC_TIER_ENGINE: &str = "treesitter"; + +/// Surface a slow single-file engine run, noting the file, its size, the backend, +/// and the elapsed time. For a wrapped upstream tool (anything but the generic +/// tree-sitter tier) the message suggests reporting it upstream, since the cost +/// is in that tool, not in polylint. +fn note_slow_engine( + path: &std::path::Path, + bytes: usize, + engine: &str, + elapsed: std::time::Duration, +) { + // tracing's Value impls cover u64 but not u128/usize, so normalise here. + let bytes = bytes as u64; + let elapsed_ms = elapsed.as_millis() as u64; + let file = path.display(); + if elapsed >= SLOW_ENGINE_WARN { + if engine == GENERIC_TIER_ENGINE { + tracing::warn!( + %file, + bytes, + engine, + elapsed_ms, + "slow on a large file (generic tree-sitter tier)" + ); + } else { + tracing::warn!( + %file, + bytes, + engine, + elapsed_ms, + "slow on this file; the cost is in the `{engine}` backend — \ + consider reporting it upstream to the {engine} project" + ); + } + } else if elapsed >= SLOW_ENGINE_INFO { + tracing::info!( + %file, + bytes, + engine, + elapsed_ms, + "engine spent notable time on this file" + ); + } +} + /// Apply autofix edit groups to `content`, one group per diagnostic. /// /// Each group is the full `fix` vec of one [`Diagnostic`] and is applied @@ -523,18 +605,23 @@ fn format_one( content: Arc::clone(¤t), }; let engine_plans = plans.get(&f.language).map(Vec::as_slice).unwrap_or(&[]); + let cache_enabled = cache.enabled(); for plan in engine_plans { // Each engine's output feeds the next, so the digest is recomputed from // the current text; the args, however, were serialised once per engine. - let digest = ResultCache::single_file_digest(¤t); - let key = ResultCache::key_with_args( - Namespace::Fmt, - plan.engine.name(), - plan.engine.version(), - &plan.serialized_args, - &digest, - ); - if let Some(bytes) = cache.get(Namespace::Fmt, &key) + // Only needed to address the cache, so skip it when the cache is off. + let key = cache_enabled.then(|| { + let digest = ResultCache::single_file_digest(¤t); + ResultCache::key_with_args( + Namespace::Fmt, + plan.engine.name(), + plan.engine.version(), + &plan.serialized_args, + &digest, + ) + }); + if let Some(key) = &key + && let Some(bytes) = cache.get(Namespace::Fmt, key) && let Ok(text) = String::from_utf8(bytes) { push_engine_debug(debug.as_mut(), plan, None); @@ -542,14 +629,20 @@ fn format_one( continue; } src.content = Arc::clone(¤t); - // Time the engine only when debug collection is on (zero cost otherwise). - let started = collect_debug.then(std::time::Instant::now); + let started = std::time::Instant::now(); let out: Arc = match plan.engine.format(&src, &plan.config)? { FormatOutput::Unchanged => Arc::clone(¤t), FormatOutput::Formatted(s) => Arc::from(s), }; - push_engine_debug(debug.as_mut(), plan, started); - if let Err(error) = cache.put(Namespace::Fmt, &key, out.as_bytes()) { + let elapsed = started.elapsed(); + note_slow_engine(&f.path, src.content.len(), plan.engine.name(), elapsed); + push_engine_debug(debug.as_mut(), plan, Some(started)); + // Persist only results expensive enough that reloading beats recomputing + // (see MIN_CACHE_DURATION). + if let Some(key) = &key + && elapsed >= MIN_CACHE_DURATION + && let Err(error) = cache.put(Namespace::Fmt, key, out.as_bytes()) + { tracing::warn!( engine = plan.engine.name(), "failed to store fmt cache entry: {error:#}" @@ -621,6 +714,91 @@ fn configure_pool(jobs: Option) { mod tests { use super::*; + /// A lint engine that sleeps a fixed duration, used to exercise the + /// duration-gated cache-write policy ([`MIN_CACHE_DURATION`]). + struct TimedEngine { + name: &'static str, + delay: std::time::Duration, + } + + impl Engine for TimedEngine { + fn name(&self) -> &'static str { + self.name + } + fn languages(&self) -> &'static [Language] { + &[] + } + fn capabilities(&self) -> crate::engine::Capabilities { + crate::engine::Capabilities { + lint: true, + format: false, + fix: false, + } + } + fn version(&self) -> &str { + "1" + } + fn lint(&self, _src: &SourceFile, _cfg: &EngineConfig) -> anyhow::Result> { + std::thread::sleep(self.delay); + Ok(Vec::new()) + } + } + + /// Only results whose engine ran at least [`MIN_CACHE_DURATION`] are + /// persisted: a cheap result is recomputed each run (caching it cost more + /// than recomputing), while an expensive one is cached even when it produced + /// no diagnostics (the gate is on cost, not on emptiness). + #[test] + fn caches_only_results_above_duration_threshold() { + let tmp = tempfile::tempdir().unwrap(); + let cache = ResultCache::open(tmp.path().join("cache"), true).expect("open cache"); + let config = Config::default(); + let content = "x = 1\n"; + let language = Language::Python; + + let empty_args = ResultCache::serialize_args(&toml::Table::new()); + let key_for = |name: &str| { + let digest = ResultCache::single_file_digest(content); + ResultCache::key_with_args(Namespace::Lint, name, "1", &empty_args, &digest) + }; + let plan = |name: &'static str, delay| EnginePlan { + engine: Box::new(TimedEngine { name, delay }), + config: config.engine_config(&language, name, Kind::Lint), + serialized_args: ResultCache::serialize_args(&toml::Table::new()), + }; + let file = DiscoveredFile { + path: std::path::PathBuf::from("mem.py"), + language: language.clone(), + }; + + // Fast engine (no delay): below threshold → must not be cached. + let mut plans: FxHashMap> = FxHashMap::default(); + plans.insert( + language.clone(), + vec![plan("fast-eng", std::time::Duration::ZERO)], + ); + lint_content(&file, &plans, &cache, content, false).expect("lint fast"); + assert!( + cache.get(Namespace::Lint, &key_for("fast-eng")).is_none(), + "a sub-threshold (cheap) result must not be cached" + ); + + // Slow engine (> threshold): must be cached despite producing no diagnostics. + let mut plans: FxHashMap> = FxHashMap::default(); + plans.insert( + language.clone(), + vec![plan( + "slow-eng", + MIN_CACHE_DURATION + std::time::Duration::from_millis(20), + )], + ); + lint_content(&file, &plans, &cache, content, false).expect("lint slow"); + assert!( + cache.get(Namespace::Lint, &key_for("slow-eng")).is_some(), + "an above-threshold (expensive) result must be cached" + ); + } + #[test] fn recognizes_generated_lock_files() { for name in [