diff --git a/.agents/skills/gpui-bench/SKILL.md b/.agents/skills/gpui-bench/SKILL.md new file mode 100644 index 0000000000..8aa142b836 --- /dev/null +++ b/.agents/skills/gpui-bench/SKILL.md @@ -0,0 +1,387 @@ +--- +name: gpui-bench +description: >- + Design, write, review, run, and interpret production-shaped GPUI Criterion + benchmarks, including gpui::bench, BenchAppContext, renderer and task + benchmarks, headless Metal frame data, responsiveness and hang regressions, + feature isolation from test-support, and before/after performance evidence. +--- + +# GPUI Benchmarks + +Use this skill when a user asks to benchmark GPUI code, reproduce a UI hang or frame drop, evaluate a performance fix, use `#[gpui::bench]`, interpret `BenchReport`, or review whether a GPUI benchmark represents production. + +The primary goal is UI responsiveness. Throughput matters, but a UI that finishes work quickly while blocking input and frames is still regressed. + +## Start with the performance question + +Before editing, establish or derive: + +1. What user-visible problem is being reproduced: slow computation, long foreground poll, delayed input, frame drops, scrolling hitch, GPU draw cost, or a true hang? +2. What production event starts the work, and what path performs it? +3. What competing UI work must remain responsive? Prefer a rendered progress indicator, cursor, spinner, or scrolling frame when practical. +4. What target frame rate applies? GPUI defaults to 120 FPS, an 8.33 ms frame budget. At 60 FPS the budget is 16.67 ms. +5. What fixed workload sizes expose a progression from normal to degraded to severe? +6. What state proves the workload completed without dropping, duplicating, or reordering work? +7. Which commits are the baseline and candidate, and can the exact same benchmark code run on both? + +Ask only for inputs that cannot be derived from the repository, issue, trace, or existing benchmark. + +## Non-negotiable rules + +- A benchmark must not enable any crate's `test-support` feature, directly or transitively. +- Use production constructors, storage, executors, rendering, synchronization, and data sizes whenever practical. +- Do not use `TestAppContext`, deterministic test executors, reduced test-only CRDT settings, fake clocks, fake filesystems, or fake services merely because setup is easier. +- A narrow benchmark seam may simulate an external boundary such as a PTY, server, or filesystem event, but everything after that boundary should follow the production path. +- A performance fix should normally include or extend a benchmark that reproduces its problem. If it does not, explain why before implementing the fix. +- Every benchmark or profile invocation run by an agent must have a hard timeout of at most five minutes. Use `timeout_ms <= 300000` with the terminal tool. +- Run smoke and quick modes before measured runs. Never start an unbounded profile or Criterion run. +- Do not run measured benchmarks concurrently. Parallel benchmark processes contaminate each other's results. +- Preserve correctness assertions in benchmark fixtures. Faster output that loses work is not an improvement. +- Never invent timing, frame, percentile, or throughput results. + +## Verify feature isolation first + +Use GPUI's canonical `bench-support` feature. The published `bench` feature is a temporary compatibility alias; new code should not use it. Neither feature may enable `test-support`. + +Inspect the complete feature graph for the actual benchmark package: + +```sh +feature_tree="$(cargo tree -p -e normal,build,dev,features)" +if grep -F 'feature "test-support"' <<<"${feature_tree}"; then + echo 'benchmark graph contains test-support' >&2 + exit 1 +fi +``` + +Also use inverse feature trees when diagnosing a leak: + +```sh +cargo tree -p -e features -i gpui +cargo tree -p -e features -i +``` + +Cargo unifies features across every target in one package. If an unrelated benchmark needs `test-support`, move the production benchmark to a separate package when necessary; selecting one `--bench` target does not undo package-wide feature unification. + +Add a repository script or CI check for important isolation rules. Do not rely only on a one-time manual `cargo tree` inspection. + +## Benchmark-only APIs + +Use these feature names: + +- `bench-support`: benchmark infrastructure and narrow, production-faithful constructors or external-boundary seams. +- `test-support`: test doubles, mutation hooks, failure injection, and test harnesses. It is forbidden in benchmark graphs. + +The `#[gpui::bench]` attribute keeps its existing name. The Cargo feature describes support consumed by benchmark targets, not the benchmark declaration itself. + +When a useful function is currently test-gated: + +1. Determine whether its behavior is production-safe or whether it is a fake. +2. Extract shared production logic into a private primitive when possible. +3. Expose the smallest wrapper under `#[cfg(any(test, feature = "bench-support"))]`. +4. Keep mutations, failure injection, and fake global state test-only unless the benchmark specifically models that production boundary. +5. Add a parity test when the benchmark wrapper must match an existing test helper. +6. Verify that enabling `bench-support` alone brings in no `test-support` feature. + +Prefer a seam that supplies bytes or events to a production pipeline over widening an entire fake test harness. + +## Model the production path + +Map the workload before constructing the fixture: + +```text +production trigger + -> queue or event boundary + -> background preparation + -> foreground application + -> invalidation + -> layout + -> prepaint + -> paint + -> present + -> observable completion +``` + +Keep each relevant boundary in the benchmark. For a queue or stream, drive enough work to exceed capacity and let a concurrent producer refill it while the consumer drains. A burst that fits entirely in the queue cannot reproduce sustained backpressure. + +Use realistic expensive shapes, not only easy append-only inputs. For rendering, include representative text lengths, styles, images, tools, comments, viewport sizes, and invalidation patterns. Change one workload dimension at a time so scaling remains interpretable. + +Separate each iteration into: + +1. **Prepare:** construct or reset state outside timing. +2. **Measure:** perform the production operation. +3. **Validate:** assert output, work counts, ordering, and completion outside timing when possible. + +State whether caches, glyph atlases, storage, and other fixtures are cold or warm. Do not accidentally compare a cold baseline with a warm candidate. + +## Choose the right GPUI measurement API + +`#[gpui::bench]` creates a Criterion benchmark using `BenchAppContext` and a production-style threaded dispatcher. The benchmark function itself is synchronous; use the context's task APIs for asynchronous work. + +The attribute supports options such as: + +```rust +#[gpui::bench( + fps = 120, + inputs = workload_sizes(), + input_name = "items", + group = "Streaming update", + sample_size = 10 +)] +``` + +Inspect the current macro before depending on an option because the API is evolving. + +### `bench_iter`: synchronous functions and compute work + +Use `bench_iter` for synchronous application or compute code: + +```rust +#[gpui::bench(inputs = sizes(), input_name = "bytes", group = "Parse")] +fn parse_document(byte_count: &usize, cx: &mut gpui::BenchAppContext) { + let input = build_input(*byte_count); + cx.bench_iter(|_| { + std::hint::black_box(parse(std::hint::black_box(&input))); + }); +} +``` + +Build reusable fixtures outside the measured closure. Use `black_box` for pure computation whose result could otherwise be optimized away. If the code does not interact with GPUI, ordinary Criterion may be simpler. + +### `bench_task`: asynchronous work to completion + +Use `bench_task` when the measured operation returns a GPUI `Task` and setup can be reused: + +```rust +cx.bench_task(|cx| fixture.run_one_workload(cx)); +``` + +This captures foreground task polls, action handlers, input dispatches, and draws produced while the task runs. It can surface a long foreground poll even when no window manages to draw during the stall. + +### `bench_batched_task`: per-iteration setup outside timing + +Use `bench_batched_task` when each iteration needs fresh input or state: + +```rust +cx.bench_batched_task( + |cx| fixture.prepare_iteration(cx), + |iteration, cx| iteration.run(cx), +); +``` + +The setup result and task output are dropped outside the timed operation. This is the preferred shape for queues, streams, synchronization, and workloads whose setup must not contaminate measurement. + +### `bench_renderer`: frame production + +Use `bench_renderer` when each iteration updates an entity in a window's render tree and should draw and present a frame: + +```rust +let mut window = cx.add_empty_window(); +let view = window.update(|window, cx| { + window.replace_root(cx, |_window, cx| cx.new(|_| BenchmarkView::new())) +}); + +cx.bench_renderer(view, |view, _window, cx| { + view.advance_one_step(); + cx.notify(); +}); +``` + +Do not benchmark a detached entity and call the result rendering performance. Do not call `run_until_idle` inside a measured loop merely to make the fixture pass: production does not drain all asynchronous work before every frame, and doing so erases the scheduling behavior being measured. + +## Headless rendering and frame data + +`#[gpui::bench]` constructs its platform through `gpui::bench_platform` and requests `gpui_platform::current_headless_renderer`. + +On macOS, the headless renderer uses Metal without showing a window. `bench_renderer` shapes text with the platform text system, builds the scene, exercises the sprite atlas, and submits the frame to the GPU on present. This catches CPU rendering, glyph, atlas, scene, and GPU-submission regressions that a no-op renderer misses. + +On platforms without a headless renderer, GPUI still performs CPU-side window work, but presenting discards the scene and does not measure real GPU submission. State this limitation in results. + +Depending on the pinned revision, `BenchReport` can include: + +- dirty-to-draw duration; +- draw and present intervals; +- invalidations per frame; +- foreground task, action, and input-dispatch durations; +- p50, p90, p95, p99, and maximum durations; +- total and maximum frame-budget overruns. + +Frame-budget overruns are a synthetic missed-deadline proxy because the headless harness has no display vsync. Do not call them literal display-dropped frames. Supplement important results with app automation and an Instruments, xctrace, miniprof, or equivalent profile. + +The report may include Criterion warmup and calibration events even though Criterion's timing estimate uses its measurement phase. Read the current `BenchReport` implementation before interpreting its sample count or percentiles. + +## Responsiveness benchmarks + +A throughput-only benchmark is insufficient for foreground UI work. Measure both: + +1. **Responsiveness:** longest uninterrupted foreground poll, high-percentile foreground duration, frame-budget overruns, dirty-to-draw delay, and frame/present cadence. +2. **Completion:** total time to consume the workload and, where useful, items or bytes per second. + +A strong workload keeps an independent UI signal alive while expensive work runs. Suitable signals include a loading icon, cursor blink, scrolling viewport, progress counter, or small animation. The heavy workload and signal must share the production foreground executor so starvation is observable. + +For each fixed input size, report: + +- workload units: bytes, messages, edits, rows, frames, or tool calls; +- p50/p95/p99/max foreground work; +- total and maximum frame-budget overruns; +- draw and dirty-to-draw percentiles when a window is involved; +- total completion time; +- correctness and progress counts. + +A fix that improves maximum foreground latency while making completion slightly slower may be a valid responsiveness tradeoff. State the tradeoff rather than hiding either metric. + +## Reproducing hangs safely + +A benchmark should reproduce a hang as a long or starved operation, not leave the process stuck forever. + +For queue or streaming hangs: + +- Drive more work than the queue capacity. +- Ensure a concurrent producer can refill the queue while the consumer drains it. +- Use the production blocking, wake, gather, synchronization, and rendering path. +- Include expensive update shapes, not only small append-only inputs. +- Add fixed small, medium, and severe workloads to show the scaling curve and point where frame budgets are missed. +- Record work counts alongside durations so a faster run cannot hide dropped work. +- Assert final state, ordering, and completion. +- Use a short internal watchdog for a true deadlock and the five-minute outer command timeout. + +When possible, share the fixture between: + +- a production-shaped benchmark that measures the performance progression; and +- a deterministic regression test that asserts the underlying invariant, work bound, or eventual progress. + +Avoid fixed wall-clock performance assertions in portable unit tests. Prefer deterministic visit counts, batch bounds, no-loss assertions, or scaling properties. Use benchmark history or controlled performance CI for timing regressions. + +A smoke mode should execute the complete benchmark path once and verify correctness without doing a long statistical run. + +## Measure before optimizing + +Profile before choosing an optimization. First reduce total work or improve the algorithm and data structures. Then consider allocations, cloning, cache behavior, and only finally specialized techniques such as SIMD. + +For a hot operation, ask: + +1. Can it be called fewer times by coalescing or incremental indexing? +2. Can it process less data by staying viewport- or change-bounded? +3. Can it avoid allocation, cloning, format conversion, or repeated lookup? +4. Can its data layout improve locality? +5. Does a profile still show a compute-heavy inner loop suitable for vectorization? + +Do not infer a hot leaf from one stack or optimize a nearby function because it looks expensive. Profiles show where time is spent; the benchmark proves whether changing that work improves the user-visible result. + +Parameterize workload sizes and report throughput units when they clarify scaling. Criterion estimates include confidence intervals and outlier analysis; report raw ranges and percentiles rather than only saying a result improved. Measurement time can exceed its configured value when one iteration is long, so retain the outer five-minute timeout. + +## Before-and-after method + +1. Add the benchmark before changing the production rule when practical. +2. Prove it reproduces the symptom on the baseline. +3. Keep benchmark code identical between baseline and candidate. Use a benchmark-only commit that can be applied to both revisions or a separate comparison worktree. +4. Build both with the same optimized profile, features, Rust flags, and lockfile. +5. Run on the same machine under similar thermal and power conditions. +6. Run a short smoke first, then a bounded measured run with the same warmup, measurement time, sample size, and input filter. +7. If results are noisy, alternate baseline and candidate runs instead of running every baseline first. +8. Report raw before/after numbers and percentage changes. Keep regressions and tradeoffs visible. +9. Profile when the benchmark shows a change but does not explain the hot path. + +Do not treat a benchmark that only passes on the fixed branch as valid before understanding why it cannot run on the baseline. + +## Profile with Instruments and xctrace + +Use the normal optimized `release-fast` build for benchmark timing. For attribution-focused profiling on macOS, build the exact target with linker deduplication disabled: + +```sh +RUSTFLAGS="-C link-arg=-Wl,-no_deduplicate -C codegen-units=1" \ + cargo bench -p --bench \ + --features bench-support --profile release-fast --no-run +``` + +These flags improve Rust symbol attribution but change code generation. Do not use that build for baseline timing numbers. + +Run `dsymutil` on the exact executable printed by Cargo: + +```sh +dsymutil target/release-fast/deps/- +``` + +Prefer the `zed-bench` Instruments template, which combines Time Profiler data, CPU counters, and hang or microhang tables. Fall back to `Time Profiler` if it is unavailable: + +```sh +rm -rf /tmp/zed-profiles/.trace +xctrace record \ + --template "zed-bench" \ + --output /tmp/zed-profiles/.trace \ + --launch -- target/release-fast/deps/- \ + --bench --profile-time 10 +``` + +Running a Criterion executable directly requires `--bench`. `--profile-time` repeats the measured routine without Criterion's statistical analysis, making profiler samples easier to interpret. + +Inspect the trace table of contents and export only focused tables such as `time-sample`; never load a complete `.trace` or large XML export into agent context. Filter by target PID, main thread when investigating foreground stalls, running state, and timer-fired samples. Symbolicate with the exact binary, dSYM, architecture, and load address using `atos`, then use `rustfilt` for Rust names. + +If the profile contains a large `` bucket or implausible functions, stop treating its symbol-level attribution as actionable and recapture with `-Wl,-no_deduplicate`. Report both flat leaf and inclusive bottom-up profiles. Load the `xctrace-rust-profile` skill for the complete export and symbolication workflow. + +Trace bundles can contain process environments, paths, source, and credentials. Keep them local, inspect focused exports, and never paste raw trace metadata into a shared thread. + +## Using agents well + +Delegate independent analysis, not competing benchmark measurements. + +Useful parallel roles: + +- **Production-path auditor:** maps the real entry point, queues, executors, storage, rendering, and completion signal; read-only. +- **Feature-isolation auditor:** proves the graph contains no `test-support` and identifies the smallest `bench-support` seams; read-only. +- **Workload designer:** derives realistic fixed inputs from the incident, trace, or production data shapes and defines correctness checks; read-only. +- **Profile analyst:** analyzes an existing xctrace or miniprof without loading the raw profile into model context. + +Keep one owner for benchmark implementation and measured runs. Do not let two agents modify the same fixture or run CPU or GPU measurements concurrently. + +Every delegated task must state: + +- exact branch/base and files in scope; +- the production path and user-visible symptom; +- the prohibition on `test-support`; +- the five-minute timeout for every benchmark invocation; +- required correctness checks and metrics; +- whether the task is read-only or may edit; +- that no PR should be opened until the benchmark reproduces the baseline and validates the candidate. + +Ask agents to retain large trace exports and benchmark logs on disk and report only focused aggregates. + +## Review checklist + +Before accepting a GPUI benchmark, verify: + +- [ ] The benchmark states the user-visible performance hypothesis. +- [ ] The exact benchmark feature graph has zero `test-support` occurrences. +- [ ] Any `bench-support` seam is narrow and production-faithful. +- [ ] The benchmark uses production storage, executors, and render paths where they affect the result. +- [ ] Setup is excluded from the timed interval unless setup is the operation being measured. +- [ ] Inputs are deterministic, fixed, realistic, and include a severe case. +- [ ] Cache state is controlled and documented. +- [ ] A responsiveness signal competes with the heavy work when foreground starvation matters. +- [ ] Frame metrics and completion throughput are both reported. +- [ ] Final correctness, ordering, and work counts are asserted. +- [ ] The workload completes under bounded smoke and measured runs. +- [ ] The same benchmark code runs on baseline and candidate. +- [ ] Results include raw percentiles, maxima, and confidence information, and disclose regressions. +- [ ] macOS headless Metal coverage or non-macOS limitations are stated. +- [ ] A deterministic regression test covers the non-timing invariant where practical. +- [ ] Benchmark and profile commands are bounded to five minutes and not run concurrently. + +## Reporting results + +Lead with whether the user-visible responsiveness problem was reproduced and whether the candidate fixed it. Then provide: + +1. Exact commits, command, profile, platform, FPS, inputs, and sample configuration. +2. Foreground and frame metrics plus total completion before and after. +3. Correctness and work-count evidence. +4. Feature-isolation evidence. +5. Headless-renderer limitations and sources of noise. +6. Remaining expensive work and the next profile or optimization to pursue. + +## References + +- [Criterion measurement analysis](https://bheisler.github.io/criterion.rs/book/analysis.html) +- [Criterion advanced configuration](https://bheisler.github.io/criterion.rs/book/user_guide/advanced_configuration.html) +- [Criterion external profiling](https://bheisler.github.io/criterion.rs/book/user_guide/profiling.html) +- [The Rust Performance Book: Profiling](https://nnethercote.github.io/perf-book/profiling.html) +- [The Rust Performance Book: General Tips](https://nnethercote.github.io/perf-book/general-tips.html) diff --git a/crates/benchmarks/Cargo.toml b/crates/benchmarks/Cargo.toml index e8f4d1834e..f046d8596d 100644 --- a/crates/benchmarks/Cargo.toml +++ b/crates/benchmarks/Cargo.toml @@ -23,7 +23,7 @@ assets.workspace = true criterion.workspace = true editor = { workspace = true, features = ["test-support"] } futures.workspace = true -gpui = { workspace = true, features = ["bench"] } +gpui = { workspace = true, features = ["bench-support"] } gpui_platform = { workspace = true, features = ["test-support", "font-kit"] } itertools.workspace = true language = { workspace = true, features = ["test-support"] } diff --git a/crates/benchmarks/src/benchmarks.rs b/crates/benchmarks/src/benchmarks.rs index 4c4134b5b9..4c55af7c66 100644 --- a/crates/benchmarks/src/benchmarks.rs +++ b/crates/benchmarks/src/benchmarks.rs @@ -1,8 +1,8 @@ //! Benchmark targets for Zed crates. //! //! Benchmarks live in their own crate so benchmark-only dependencies -//! (Criterion, `gpui_platform`, gpui's `bench` feature, ...) don't weigh down -//! the test builds of the crates being benchmarked. Each file in `benches/` -//! targets one area of the codebase. +//! (Criterion, `gpui_platform`, GPUI's `bench-support` feature, ...) don't +//! weigh down the test builds of the crates being benchmarked. Each file in +//! `benches/` targets one area of the codebase. pub mod bench_utils; diff --git a/crates/gpui/Cargo.toml b/crates/gpui/Cargo.toml index 13cd4e8ef6..86d1460c93 100644 --- a/crates/gpui/Cargo.toml +++ b/crates/gpui/Cargo.toml @@ -26,7 +26,9 @@ test-support = [ "x11", "proptest", ] -bench = ["test-support", "profiler", "dep:criterion"] +bench-support = ["profiler", "dep:criterion"] +# Preserve the published feature name while consumers migrate to `bench-support`. +bench = ["bench-support"] inspector = ["gpui_macros/inspector"] leak-detection = ["backtrace"] wayland = [] diff --git a/crates/gpui/src/app.rs b/crates/gpui/src/app.rs index 0c58c55e8a..7182f96714 100644 --- a/crates/gpui/src/app.rs +++ b/crates/gpui/src/app.rs @@ -24,7 +24,7 @@ use parking_lot::RwLock; use slotmap::SlotMap; pub use async_context::*; -#[cfg(feature = "bench")] +#[cfg(feature = "bench-support")] pub use bench_context::{BenchAppContext, BenchReport, BenchWindowContext, bench_platform}; use collections::{FxHashMap, FxHashSet, HashMap, TypeIdHashMap, TypeIdHashSet, VecDeque}; pub use context::*; @@ -60,7 +60,7 @@ use crate::{ }; mod async_context; -#[cfg(feature = "bench")] +#[cfg(feature = "bench-support")] mod bench_context; mod context; mod entity_map; @@ -1702,7 +1702,7 @@ impl App { } } } else { - #[cfg(any(test, feature = "test-support", feature = "bench"))] + #[cfg(any(test, feature = "test-support", feature = "bench-support"))] for window in self .windows .values() diff --git a/crates/gpui/src/app/bench_context.rs b/crates/gpui/src/app/bench_context.rs index 7fc5831f72..278f26cb31 100644 --- a/crates/gpui/src/app/bench_context.rs +++ b/crates/gpui/src/app/bench_context.rs @@ -527,7 +527,11 @@ impl<'a, 'measurement> BenchAppContext<'a, 'measurement> { ); let foreground_executor = platform.foreground_executor(); let asset_source = Arc::new(()); - let http_client = http_client::FakeHttpClient::with_404_response(); + // Benchmark setup must not make accidental network requests. The + // production `BlockedHttpClient` reports them without enabling a + // configurable test double through `test-support`. + let http_client: Arc = + Arc::new(http_client::BlockedHttpClient::new()); let app = App::new_app(platform, asset_source, http_client); Self { diff --git a/crates/gpui/src/gpui.rs b/crates/gpui/src/gpui.rs index 927f6344e6..41e3c4733e 100644 --- a/crates/gpui/src/gpui.rs +++ b/crates/gpui/src/gpui.rs @@ -43,7 +43,8 @@ pub mod profiler; target_os = "windows", target_os = "linux", target_family = "wasm", - feature = "test-support" + feature = "test-support", + feature = "bench-support" ))] #[expect(missing_docs)] pub mod queue; diff --git a/crates/gpui/src/platform.rs b/crates/gpui/src/platform.rs index 16ca199d90..44b0480617 100644 --- a/crates/gpui/src/platform.rs +++ b/crates/gpui/src/platform.rs @@ -9,10 +9,10 @@ pub mod layer_shell; /// Types for configuring parent-anchored popup windows such as menus, dropdowns and tooltips. pub mod popup; -#[cfg(any(test, feature = "test-support"))] +#[cfg(any(test, feature = "test-support", feature = "bench-support"))] mod threaded_dispatcher; -#[cfg(any(test, feature = "test-support"))] +#[cfg(any(test, feature = "test-support", feature = "bench-support"))] mod test; #[cfg(all(target_os = "macos", any(test, feature = "test-support")))] @@ -47,7 +47,7 @@ use anyhow::bail; use anyhow::{Context as _, Result}; use async_task::Runnable; use futures::channel::oneshot; -#[cfg(any(test, feature = "test-support"))] +#[cfg(any(test, feature = "test-support", feature = "bench-support"))] use image::RgbaImage; use image::codecs::gif::GifDecoder; use image::{AnimationDecoder as _, DynamicImage, Frame}; @@ -78,13 +78,13 @@ pub use app_menu::*; pub use keyboard::*; pub use keystroke::*; -#[cfg(any(test, feature = "test-support"))] +#[cfg(any(test, feature = "test-support", feature = "bench-support"))] pub(crate) use test::*; #[cfg(any(test, feature = "test-support"))] pub use test::{TestDispatcher, TestScreenCaptureSource, TestScreenCaptureStream}; -#[cfg(any(test, feature = "test-support"))] +#[cfg(any(test, feature = "test-support", feature = "bench-support"))] pub use threaded_dispatcher::ThreadedDispatcher; #[cfg(all(target_os = "macos", any(test, feature = "test-support")))] @@ -974,7 +974,7 @@ pub trait PlatformWindow: HasWindowHandle + HasDisplayHandle { /// Inform the adapter of updated window bounds. fn a11y_update_window_bounds(&self) {} - #[cfg(any(test, feature = "test-support"))] + #[cfg(any(test, feature = "test-support", feature = "bench-support"))] fn as_test(&mut self) -> Option<&mut TestWindow> { None } @@ -989,7 +989,7 @@ pub trait PlatformWindow: HasWindowHandle + HasDisplayHandle { } /// A renderer for headless windows that can produce real rendered output. -#[cfg(any(test, feature = "test-support"))] +#[cfg(any(test, feature = "test-support", feature = "bench-support"))] pub trait PlatformHeadlessRenderer { /// Render a scene and return the result as an RGBA image. fn render_scene_to_image( @@ -1055,14 +1055,14 @@ pub trait PlatformDispatcher: Send + Sync { gpui_util::defer(Box::new(|| {})) } - #[cfg(any(test, feature = "test-support"))] + #[cfg(any(test, feature = "test-support", feature = "bench-support"))] fn as_test(&self) -> Option<&TestDispatcher> { None } // This cfg must match the `threaded_dispatcher` module's, which implements // this method whenever it compiles. - #[cfg(any(test, feature = "test-support"))] + #[cfg(any(test, feature = "test-support", feature = "bench-support"))] fn as_threaded(&self) -> Option<&ThreadedDispatcher> { None } @@ -1331,7 +1331,7 @@ pub trait PlatformAtlas { ) -> Result>; fn remove(&self, key: &AtlasKey); - #[cfg(any(test, feature = "test-support"))] + #[cfg(any(test, feature = "test-support", feature = "bench-support"))] fn contains(&self, _key: &AtlasKey) -> bool { false } diff --git a/crates/gpui/src/platform/test.rs b/crates/gpui/src/platform/test.rs index 9227df5b63..a327d8fe94 100644 --- a/crates/gpui/src/platform/test.rs +++ b/crates/gpui/src/platform/test.rs @@ -8,4 +8,5 @@ pub(crate) use display::*; pub(crate) use platform::*; pub(crate) use window::*; +#[cfg(any(test, feature = "test-support"))] pub use platform::{TestScreenCaptureSource, TestScreenCaptureStream}; diff --git a/crates/gpui/src/platform/test/platform.rs b/crates/gpui/src/platform/test/platform.rs index 86c2297d84..5e82fbe3d8 100644 --- a/crates/gpui/src/platform/test/platform.rs +++ b/crates/gpui/src/platform/test/platform.rs @@ -1,12 +1,17 @@ +#[cfg(any(test, feature = "test-support"))] +use crate::NoopTextSystem; +#[cfg(any(test, feature = "test-support"))] +use crate::PathPromptOptions; use crate::{ AnyWindowHandle, BackgroundExecutor, ClipboardItem, CursorStyle, DevicePixels, - DummyKeyboardMapper, ForegroundExecutor, Keymap, NoopTextSystem, PathPromptOptions, Platform, - PlatformDisplay, PlatformHeadlessRenderer, PlatformKeyboardLayout, PlatformKeyboardMapper, - PlatformTextSystem, PromptButton, ScreenCaptureFrame, ScreenCaptureSource, ScreenCaptureStream, - SharedString, SourceMetadata, SystemNotification, SystemNotificationResponse, Task, - TestDisplay, TestWindow, ThermalState, WindowAppearance, WindowParams, size, + DummyKeyboardMapper, ForegroundExecutor, Keymap, Platform, PlatformDisplay, + PlatformHeadlessRenderer, PlatformKeyboardLayout, PlatformKeyboardMapper, PlatformTextSystem, + PromptButton, ScreenCaptureFrame, ScreenCaptureSource, ScreenCaptureStream, SharedString, + SourceMetadata, SystemNotification, SystemNotificationResponse, Task, TestDisplay, TestWindow, + ThermalState, WindowAppearance, WindowParams, size, }; use anyhow::Result; +#[cfg(any(test, feature = "test-support"))] use collections::VecDeque; use futures::channel::oneshot; use parking_lot::Mutex; @@ -30,6 +35,7 @@ pub(crate) struct TestPlatform { current_primary_item: Mutex>, #[cfg(target_os = "macos")] current_find_pasteboard_item: Mutex>, + #[cfg(any(test, feature = "test-support"))] pub(crate) prompts: RefCell, screen_capture_sources: RefCell>, pub opened_url: RefCell>, @@ -77,6 +83,7 @@ impl ScreenCaptureStream for TestScreenCaptureStream { } } +#[cfg(any(test, feature = "test-support"))] struct TestPrompt { msg: String, detail: Option, @@ -93,6 +100,7 @@ pub(crate) struct TestSystemNotifications { response_callback: Option>, } +#[cfg(any(test, feature = "test-support"))] #[derive(Default)] pub(crate) struct TestPrompts { multiple_choice: VecDeque, @@ -104,6 +112,7 @@ pub(crate) struct TestPrompts { } impl TestPlatform { + #[cfg(any(test, feature = "test-support"))] pub fn new(executor: BackgroundExecutor, foreground_executor: ForegroundExecutor) -> Rc { Self::with_platform( executor, @@ -113,6 +122,7 @@ impl TestPlatform { ) } + #[cfg(any(test, feature = "test-support"))] pub fn with_text_system( executor: BackgroundExecutor, foreground_executor: ForegroundExecutor, @@ -132,6 +142,7 @@ impl TestPlatform { Rc::new_cyclic(|weak| TestPlatform { background_executor: executor, foreground_executor, + #[cfg(any(test, feature = "test-support"))] prompts: Default::default(), screen_capture_sources: Default::default(), active_cursor: Default::default(), @@ -151,6 +162,7 @@ impl TestPlatform { }) } + #[cfg(any(test, feature = "test-support"))] pub(crate) fn simulate_new_path_selection( &self, select_path: impl FnOnce(&std::path::Path) -> Option, @@ -164,6 +176,7 @@ impl TestPlatform { tx.send(Ok(select_path(&path))).ok(); } + #[cfg(any(test, feature = "test-support"))] pub(crate) fn simulate_path_prompt_response( &self, select_paths: impl FnOnce(&PathPromptOptions) -> Option>, @@ -187,10 +200,12 @@ impl TestPlatform { tx.send(Ok(selection)).ok(); } + #[cfg(any(test, feature = "test-support"))] pub(crate) fn did_prompt_for_paths(&self) -> bool { !self.prompts.borrow().paths.is_empty() } + #[cfg(any(test, feature = "test-support"))] #[track_caller] pub(crate) fn simulate_prompt_answer(&self, response: &str) { let prompt = self @@ -208,10 +223,12 @@ impl TestPlatform { prompt.tx.send(ix).ok(); } + #[cfg(any(test, feature = "test-support"))] pub(crate) fn has_pending_prompt(&self) -> bool { !self.prompts.borrow().multiple_choice.is_empty() } + #[cfg(any(test, feature = "test-support"))] pub(crate) fn pending_prompt(&self) -> Option<(String, String)> { let prompts = self.prompts.borrow(); let prompt = prompts.multiple_choice.front()?; @@ -221,10 +238,14 @@ impl TestPlatform { )) } + #[cfg(any(test, feature = "test-support"))] pub(crate) fn set_screen_capture_sources(&self, sources: Vec) { *self.screen_capture_sources.borrow_mut() = sources; } + /// Queues the prompt so a test can later inspect or answer it through + /// [`Self::pending_prompt`] and [`Self::simulate_prompt_answer`]. + #[cfg(any(test, feature = "test-support"))] pub(crate) fn prompt( &self, msg: &str, @@ -245,6 +266,19 @@ impl TestPlatform { rx } + /// Benchmarks have no API to answer a prompt, so this doesn't retain it + /// for later inspection; dropping the sender immediately cancels the + /// returned receiver instead of leaving it pending indefinitely. + #[cfg(not(any(test, feature = "test-support")))] + pub(crate) fn prompt( + &self, + _msg: &str, + _detail: Option<&str>, + _answers: &[PromptButton], + ) -> oneshot::Receiver { + oneshot::channel().1 + } + pub(crate) fn set_active_window(&self, window: Option) { let executor = self.foreground_executor(); let previous_window = self.active_window.borrow_mut().take(); @@ -267,26 +301,32 @@ impl TestPlatform { .detach(); } + #[cfg(any(test, feature = "test-support"))] pub(crate) fn did_prompt_for_new_path(&self) -> bool { !self.prompts.borrow().new_path.is_empty() } + #[cfg(any(test, feature = "test-support"))] pub(crate) fn app_identity(&self) -> Option<(SharedString, SharedString)> { self.system_notifications.borrow().app_identity.clone() } + #[cfg(any(test, feature = "test-support"))] pub(crate) fn shown_system_notifications(&self) -> Vec { self.system_notifications.borrow().shown.clone() } + #[cfg(any(test, feature = "test-support"))] pub(crate) fn delivered_system_notifications(&self) -> Vec { self.system_notifications.borrow().delivered.clone() } + #[cfg(any(test, feature = "test-support"))] pub(crate) fn dismissed_system_notifications(&self) -> Vec { self.system_notifications.borrow().dismissed.clone() } + #[cfg(any(test, feature = "test-support"))] pub(crate) fn simulate_system_notification_response( &self, response: SystemNotificationResponse, @@ -424,6 +464,9 @@ impl Platform for TestPlatform { unimplemented!() } + /// Queues the prompt so a test can later answer it through + /// [`Self::simulate_path_prompt_response`]. + #[cfg(any(test, feature = "test-support"))] fn prompt_for_paths( &self, options: crate::PathPromptOptions, @@ -433,6 +476,21 @@ impl Platform for TestPlatform { rx } + /// Benchmarks have no API to answer a path prompt, so this doesn't + /// retain it for later inspection; dropping the sender immediately + /// cancels the returned receiver instead of leaving it pending + /// indefinitely. + #[cfg(not(any(test, feature = "test-support")))] + fn prompt_for_paths( + &self, + _options: crate::PathPromptOptions, + ) -> oneshot::Receiver>>> { + oneshot::channel().1 + } + + /// Queues the prompt so a test can later answer it through + /// [`Self::simulate_new_path_selection`]. + #[cfg(any(test, feature = "test-support"))] fn prompt_for_new_path( &self, directory: &std::path::Path, @@ -446,6 +504,19 @@ impl Platform for TestPlatform { rx } + /// Benchmarks have no API to answer a new-path prompt, so this doesn't + /// retain it for later inspection; dropping the sender immediately + /// cancels the returned receiver instead of leaving it pending + /// indefinitely. + #[cfg(not(any(test, feature = "test-support")))] + fn prompt_for_new_path( + &self, + _directory: &std::path::Path, + _suggested_name: Option<&str>, + ) -> oneshot::Receiver>> { + oneshot::channel().1 + } + fn can_select_mixed_files_and_dirs(&self) -> bool { true } @@ -586,6 +657,7 @@ impl Platform for TestPlatform { impl TestScreenCaptureSource { /// Create a fake screen capture source, for testing. + #[cfg(any(test, feature = "test-support"))] pub fn new() -> Self { Self {} } diff --git a/crates/gpui/src/platform/test/window.rs b/crates/gpui/src/platform/test/window.rs index 939c8a8bf8..466b965e8b 100644 --- a/crates/gpui/src/platform/test/window.rs +++ b/crates/gpui/src/platform/test/window.rs @@ -7,6 +7,7 @@ use crate::{ }; use collections::HashMap; use gpui_util::ResultExt as _; +#[cfg(any(test, feature = "test-support"))] use image::RgbaImage; use parking_lot::Mutex; use raw_window_handle::{HasDisplayHandle, HasWindowHandle}; diff --git a/crates/gpui/src/platform/threaded_dispatcher.rs b/crates/gpui/src/platform/threaded_dispatcher.rs index ff45e1c88b..762464a12e 100644 --- a/crates/gpui/src/platform/threaded_dispatcher.rs +++ b/crates/gpui/src/platform/threaded_dispatcher.rs @@ -268,7 +268,7 @@ impl ThreadedDispatcher { /// drains — deferred work that re-queues itself (idle sweeps, pollers) /// must not extend a benchmark's measured interval past the completion it /// awaits. - #[cfg(any(test, feature = "bench"))] + #[cfg(any(test, feature = "bench-support"))] pub(crate) fn run_until(&self, mut ready: impl FnMut() -> Option) -> R { assert!( self.is_main_thread(), @@ -296,7 +296,7 @@ impl ThreadedDispatcher { /// readiness between them: a task that perpetually re-queues itself (like /// an idle-time sweep) would otherwise keep [`Self::drain_main_queue`] /// looping past the completion the caller is waiting for. - #[cfg(any(test, feature = "bench"))] + #[cfg(any(test, feature = "bench-support"))] fn run_one_main_task(&self) -> bool { let runnable = self.main_receiver.lock().try_pop(); match runnable { @@ -373,7 +373,7 @@ impl ThreadedDispatcher { /// Whether no main-thread work is queued, no background or timer /// runnables are queued or running, and no armed timer is due. Timers /// that aren't due yet are ignored, as in [`Self::run_until_idle`]. - #[cfg(any(test, feature = "bench"))] + #[cfg(any(test, feature = "bench-support"))] pub(crate) fn is_idle(&self) -> bool { !self.main_queue_has_work() && !self.has_due_timer() && *self.idle.inflight.lock() == 0 } diff --git a/crates/gpui/src/profiler.rs b/crates/gpui/src/profiler.rs index f3dc0ce568..340f054b9c 100644 --- a/crates/gpui/src/profiler.rs +++ b/crates/gpui/src/profiler.rs @@ -734,10 +734,10 @@ pub fn set_trace_enabled(enabled: bool) -> bool { } } -#[cfg(any(feature = "bench", all(test, feature = "profiler")))] +#[cfg(any(feature = "bench-support", all(test, feature = "profiler")))] pub(crate) struct TraceGuard; -#[cfg(any(feature = "bench", all(test, feature = "profiler")))] +#[cfg(any(feature = "bench-support", all(test, feature = "profiler")))] pub(crate) fn trace_scope() -> TraceGuard { let incremented = TRACE_STATE.fetch_update(Ordering::AcqRel, Ordering::Acquire, |state| { (state & TRACE_SCOPE_COUNT_MASK < TRACE_SCOPE_COUNT_MASK).then_some(state + 1) @@ -746,7 +746,7 @@ pub(crate) fn trace_scope() -> TraceGuard { TraceGuard } -#[cfg(any(feature = "bench", all(test, feature = "profiler")))] +#[cfg(any(feature = "bench-support", all(test, feature = "profiler")))] impl Drop for TraceGuard { fn drop(&mut self) { let previous_state = diff --git a/crates/gpui/src/window.rs b/crates/gpui/src/window.rs index a871936dc4..e677bb9331 100644 --- a/crates/gpui/src/window.rs +++ b/crates/gpui/src/window.rs @@ -3035,7 +3035,7 @@ impl Window { /// Benchmarks drive drawing synchronously rather than through a platform /// frame-request loop, so they call this after each measured update to /// submit the frame like production presentation would. - #[cfg(any(feature = "bench", all(test, feature = "profiler")))] + #[cfg(any(feature = "bench-support", all(test, feature = "profiler")))] pub fn present_if_needed(&mut self) { if self.needs_present.get() { self.present();