Add a PyTorch cold-start "time to first index" benchmark#11
Open
kinto0 wants to merge 13 commits into
Open
Conversation
Summary: currently, someone needs to comment on a github issue before a maintainer can manually assign it to them. this rarely happens before a PR is open. Instead, we should let contributors assign themselves. Reviewed By: connernilsen Differential Revision: D109479872
issue claim bot (facebook#3918)
Sets up the scaffolding for running Pyrefly microbenchmarks under divan with CodSpeed instrumentation, so benchmark regressions can be tracked automatically on pull requests. codspeed-divan-compat is a drop-in replacement for the divan benchmarking crate: locally it runs as a normal walltime harness, while in CI (under `cargo codspeed`) it emits instrumented measurements. Because the bench macros it re-exports expand to `divan::` paths, the crate is aliased to `divan` (package = "codspeed-divan-compat") in Cargo.toml. Adds a [[bench]] (harness = false) and the divan dev-dependency on the pyrefly crate, plus the harness scaffolding in pyrefly/benches/micro.rs: a SHARED_STATE that pre-initializes stdlib so only snippet checking is measured, a check_code helper, divan::main(), and a smoke benchmark. The real benchmarks are ported in the next commit.
Adds a GitHub Actions workflow that, on pushes to main and on PRs, installs cargo-codspeed, builds the pyrefly benchmark target, and runs it through the CodSpeed action in instruction-count simulation mode. Authentication uses GitHub OpenID Connect (id-token: write), so no CODSPEED_TOKEN secret is required. Also adds a [profile.bench] that inherits release but disables LTO and uses multiple codegen units, keeping the CodSpeed build within standard CI runner memory limits, and adds a CodSpeed status badge to the README.
The symlink pointed at ../pyrefly/rust-toolchain, whose target was deleted in e1416af as redundant, leaving the symlink dangling. Swatinem/rust-cache globs for rust-toolchain files and crashes with ENOENT when it follows the broken link, breaking the CodSpeed (and eventually all rust-cache) CI jobs. rustup already resolves the toolchain by walking up to the repo-root rust-toolchain.toml, so the symlink was redundant; removing it fixes the cache step with no loss of toolchain pinning for the wasm build.
The lsp_interaction tests drive the language server through an in-process `LspInteraction`/`TestClient` harness (`object_model`). We want the Pyrefly benchmarks to exercise the same realistic LSP path, but a `benches/` target and the test target are separate compilation units, so neither can import the other's modules — the harness has to live in a library both depend on. Move `object_model` and `init` into a new dev-only crate, `pyrefly_lsp_test`, rather than into the shipping `pyrefly` library (which would require feature/cfg gymnastics and pull `pretty_assertions` into normal builds). The test target now re-exports `pyrefly_lsp_test::object_model` so its 50+ modules keep referring to `crate::object_model` unchanged; `ClientRequestHandle::id` becomes `pub` since it is now read across a crate boundary. Also drop the `#[ignore]`d `pytorch_benchmark` manual test; the next commits port it to a CodSpeed benchmark built on this shared harness.
The PyTorch error-propagation benchmark needs a large, real-world Python codebase to type-check, which isn't present on CI runners. Vendor it as a git submodule pinned to a specific PyTorch revision (under `pyrefly/benches/pytorch`) so the benchmark workload is reproducible across local runs and CI without bloating the repository with copied files.
The old `pytorch_benchmark` was an `#[ignore]`d manual test: it measured how long a type error takes to propagate across PyTorch (from `torch/nn/__init__.py` to `torch/distributed/pipelining/_backward.py`), but nothing ran it automatically, so regressions in incremental-recheck latency went untracked. Re-express it as a divan benchmark under the same `codspeed-divan-compat` harness as `micro.rs`, registered as a `[[bench]]` and run by the CodSpeed CI workflow (which now checks out the PyTorch submodule). The cold start — server launch and the first full project check — happens in `with_inputs`, outside the measured region, so only the propagation latency is timed; a fresh server per sample keeps the non-idempotent edit correct, and an RAII guard restores the edited file so the submodule working tree stays clean. The trigger rebinds `Parameter` to a non-type value instead of deleting its re-export: deleting it lets a case-insensitive filesystem resolve `Parameter` to the `parameter.py` submodule and mask the error, so the rebind makes the benchmark deterministic across platforms. When the submodule isn't checked out the benchmark skips, so `cargo bench` still works in a bare checkout.
CodSpeed's simulation mode runs the whole benchmark binary under Valgrind, which slows execution ~10-50x and serializes threads. The cold start — a full check of everything in PyTorch's `pyrefly.toml` project-includes — then stayed silent past the harness's 50s message timeout, so setup timed out before any measurement ran. Shrink and desensitize the unmeasured cold start: use `IndexingMode::None` so only the two opened files and their import closure are checked (dependency-based rechecking of open files, which is what we measure, is unaffected), wait for the files' diagnostics to merely arrive rather than for an exact error count (which varies by pyrefly version/platform), and let the harness client use much larger timeouts via a new `TestClient::set_timeouts` for these slow instrumented runs.
Codspeed pytorch bench
The existing pytorch benchmark measures incremental recheck latency, but nothing tracks the cold-start cost a developer pays when first opening a large project — the initial index/check before the IDE can answer anything. Add `cold_start_go_to_definition` alongside it: from a fresh server it initializes, opens a file deep in the dependency graph, and resolves the first cross-file go-to-definition (`from torch.nn import Parameter`). The response can only be produced once that file and its import closure have been analyzed, so the measured wall time is the time-to-first-index. A fresh server per sample makes each run a genuine cold start. Shared LSP setup is factored into `lsp_args()`.
Merging this PR will not alter performance
Performance Changes
Comparing 
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds
cold_start_go_to_definitionnext to the existingerror_propagationbench inpyrefly/benches/pytorch.rs.From a fresh server it initializes, opens
torch/distributed/pipelining/_backward.py(deep in the dependency graph), and resolves the first cross-file go-to-definition onfrom torch.nn import Parameter. That response can only be produced once the file and its import closure have been analyzed, so the measured wall time is the time-to-first-index. A fresh server per sample makes each run a genuine cold start; shared LSP setup is factored intolsp_args().Local numbers: cold-start ~3.8s, error-propagation ~150ms.
Note:
cargo codspeed runnow executes both pytorch benches under Valgrind, so CI cost increases; worth watching the first run.