Repository navigation
perf(desktop-gpui): reduce long-menu CPU time by 9–40× - #3220
Merged
richiemcilroy merged 2 commits intoOct 25, 2026
Merged
Conversation
Contributor
|
@kartikkabadi is attempting to deploy a commit to the Cap Software Inc Team on Vercel. A member of the Team first needs to authorize it. |
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
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.
Summary
Long dropdowns, including the editor font picker, currently construct and lay out every row on each redraw. This virtualizes overflowing fixed-height menus while preserving their intrinsic width, row identities and selection indices.
For synthetic 200, 500 and 1,000-entry catalogs, keyboard, hover and wheel redraws use approximately 9×, 23 to 25× and 38 to 40× less main/UI-thread CPU time, respectively. These numbers measure the component's CPU path, not native-font speed, GPU presentation, FPS or whole-editor latency.
The change is confined to
apps/desktop-gpui/src/ui/menu.rs: +370 / -53, including 193 lines of regression tests. Production code is +177 / -53, net +124. No dependency or public API changes.Behavior preserved
ScrollHandlepreserves offsets across eager and virtualized paths. Visible ranges clamp stale offsets and overscan fractional boundaries.Evidence
Validated against upstream
69caa302a5e087b688a3f83ab369799264930c73, with candidatef9fb06fb6b126d14983c0cd7006cbd6d45f7ff73. Upstream was refreshed immediately before publication and still points to that base. The candidate source is unchanged from the reviewed input.Main/UI-thread CPU time
Representative keyboard dispatch/update/draw/cleanup operations:
The latency columns are medians of the three process medians. Speedups are medians of paired ratios, so the quotient of displayed latencies can differ slightly. Across keyboard, hover and wheel operations, the case-median ranges are 8.6 to 9.8× at 200 entries, 22.7 to 25.2× at 500, and 38.0 to 40.0× at 1,000. First-open and reopen at 1,000 entries have paired median ratios of 12.5× and 13.7×, including fresh per-open measurement-cache construction.
Complete CPU results, all 30 cases
Values below 1× mean the candidate used more CPU time. Small-menu results are within the variation of unchanged-binary controls; this does not make every menu faster. Some four-entry cases add several microseconds.
Measurement method and controls
Host: Apple M1 Pro, MacBookPro18,3, 8 logical CPUs, 16 GB RAM, macOS 27.0.1. Compiler: Rust 1.95.0. Both binaries use opt-level 3, thin LTO, one codegen unit and stripped symbols, with identical flags, dependency inputs and Cap's prepared GPUI checkout. GPUI is pinned at
5d1f83d9f27a19bec1fb241dc33b42238af9cf8dwith the repository's six patches applied.The primary timer is POSIX
CLOCK_THREAD_CPUTIME_IDon the main/UI thread. It excludes OS descheduling and CPU work on other threads. A sleep control advanced wall time by approximately 55 ms while consuming approximately 46 µs of thread CPU time.The fixture uses real Menu source, GPUI TestApp input dispatch, layout, CPU scene generation and old-frame release, with NoopTextSystem and the production
check.svg. It constructs synthetic unique font-style labels rather than enumerating this host's font catalog. Opening-time and render-time item preparation are included. A fresh TestApp is used per first-open iteration; blank-window setup, fixture generation and teardown are outside the timed interval. Reopen excludes the preceding close but includes opening preparation and two draws, with a new per-open layout cache. Native font enumeration, shaping and GPU presentation are excluded from this timer.Each series uses six catalog sizes, five operations, seven samples per case and three alternating fresh-process baseline/candidate pairs: 1,260 raw sample rows. CPU samples target 20 ms and use eight warmup operations. Raw sample arithmetic, pair medians and render counts were independently recomputed for all 30 cases. Dispatch verification separately checks that input events actually caused the expected redraws. The source and executable hashes were checked before each series.
Unchanged-binary CPU controls use the same baseline executable in both positions: three process pairs, five catalog sizes, five operations and seven samples, 1,050 rows. Individual paired ratios range from 0.79 to 1.29×; case-median ratios range from 0.88 to 1.05×. This limits the precision of small effects but is far smaller than the long-menu reductions. Other host workloads, clock frequencies and thermals were uncontrolled. No CPU affinity was applied, and task-owned builds/tests were stopped during timing.
Two wall-clock series were also retained: the original local 20 ms sample target and a 100 ms target with eight warmup operations. Both show substantial scheduling noise on this shared host. Unchanged-binary wall-clock controls produced apparent individual ratios from 0.46 to 3.67×. They do not support a precise local wall-clock headline. The title uses the separate main-thread CPU series rather than selecting the largest noisy wall-clock result. No samples were discarded. The three measured series contain 3,780 rows in total, in addition to control rows.
Retained wall-clock results, all 60 cases
Original local 20 ms series:
Longer local 100 ms series:
These tables are included for transparency. Their largest ratios are not the basis of the performance claim.
Tests and native rendering
The complete
cap-gpuiapp test target passed on the refreshed base:The three new tests ran in that suite:
menu_width_uses_the_widest_actual_rowa_menu_click_survives_scrolling_away_and_back_while_heldreleasing_a_held_menu_click_outside_cancels_it_without_breaking_the_next_clickThe focused release harness also passed 25 tests. Those overlap app tests and are not added to 1,428 as unique coverage. Every one of 1,682 baseline/candidate headless geometry and interaction records matched exactly. Each baseline/candidate timing executable passed 16 dispatch checks.
Coverage includes empty/small/threshold menus, tiny/narrow viewports, all anchors, offscreen widest labels, fractional scroll offsets, bottom clamping, CJK/RTL/tab/multiline text, tall-line-height fallback, font/style/theme changes, checked-state changes, reordered/shrunk/resized menus, absolute callback indices, keyboard commit/dismiss, reopening, held presses and outside releases.
Native validation used real macOS fonts and Metal captures in an isolated offscreen component fixture, with the production SVG and 192 installed font names. The matrix is eight list sizes, three menu styles and five states: initial, fractional scroll, bottom, font/theme change and shrink. Each image is 560×420.
An earlier canvas-based measurement attempt changed scroll extent and was rejected as intrusive instrumentation. Its results are excluded. The accepted bounds comparison is independently checked against uninstrumented images.
Real editor font-picker/sidebar/toolbar, settings and screenshot callers were reviewed. This validation does not establish a full signed-in app workflow, physical-pointer/IME behavior, local Windows/Linux execution, end-user FPS or workspace-wide integration coverage. The 24 ignored tests remain opt-in paths involving physical devices, audio/GPU scenarios, external fixtures or uploads.
Build, lint and formatting
The normal app test target builds successfully with the standard prepared GPUI and native dependencies. Baseline and candidate production/test Clippy runs both exit 0 and report 49 identical existing diagnostics, zero new diagnostics, none in menu.rs. This is not a claim that the workspace passes
-D warnings.Changed-file Rust formatting and
git diff --checkpass. Workspace formatting reports differences in four untouched upstream files:feeds.rs,main.rs,onboarding_window.rsandsettings_window.rs. Those files are byte-identical to the validated base; this PR does not include unrelated formatting changes. The initial sandboxed build could not access Xcode DerivedData. Repeating it with the standard cache access succeeded, without source changes.Overlap check
Checked complete file inventories for all 162 open PRs, including every new or changed head after the initial 159-PR audit, plus historical menu/font-picker/dropdown searches and recently closed PRs. No equivalent published change was found at publication time. #2587 overlaps this file as part of a broader design-system rewrite but still eagerly renders its menu rows. It may cause future conflicts. #3125 leaves font-picker virtualization as separate work. #3103 borrows the presets store in
editor_window.rs, and #3110 restores Settings focus; neither virtualizes these rows.Reproduction
App checks, using Rust 1.95.0 and the repository's normal GPUI/native-dependency setup:
Recorded app runs used cached dependencies with
--offlineand a separate sharedCARGO_TARGET_DIR. To compare Clippy warnings, run the same command on the base and candidate, retaining JSON compiler diagnostics rather than changing lint settings.Standalone CPU benchmark source and runner
The complete fixture, build helper and paired runner are below so this measurement can be reproduced without adding benchmark files to the PR's code diff. The full local raw-data/image/log archive is kept local. This description contains all 90 summary cases, the controls and their limitations.
Create a standalone directory next to the checked-out Cap repository, with a
srcsubdirectory. Copy the following files into it. Obtain the production snapshots from the repository:Use the following manifest, adapting only the GPUI path to the checkout prepared by the repository script. The extra direct
libcdependency is confined to this standalone timing fixture and uses libc 0.2.189 in the measured build. It is not a Cap dependency change.source-provenance.json:{ "baseline_commit": "ec15b98403441aee7780ff7734fef93d2e1a323f", "files": { "baseline-menu.rs": { "source": "apps/desktop-gpui/src/ui/menu.rs", "sha256": "de1425ea4177ab97be69012ee68bb498ddef4c18ccb16945b24965044ea04852" }, "baseline-theme.rs": { "source": "apps/desktop-gpui/src/theme.rs", "sha256": "604d7dc9c674dbb6e75c2c5fb2ee3d6a12861873ea6390f01898acb18e4c3da4" }, "check.svg": { "source": "apps/desktop-gpui/assets/icons/check.svg", "sha256": "3751b37796055d12e3b955b9afb117878d82ba26d35629afd460d7b070885c17" } } }src/main.rs:build-binaries.py:run-pairs.py:run-controls.py, with only the original local directory paths adapted to this standalone directory:Generate a Cargo.lock once before building the pair, then keep it fixed. The helpers build both variants with
--lockedand record each compiler version, GPUI tree hash, source hash, profile and executable hash. A newly generated dependency lock may differ from the measured lock; record that difference rather than claiming a byte-identical environment.The runner alternates baseline/candidate order across three fresh-process pairs, retains every raw CSV, verifies sample identities/arithmetic/render counts and emits paired summaries. Its original baseline commit label describes the archive's snapshot; the baseline menu/theme/SVG were independently verified equal to the upstream validation base above. Run matched controls with the same baseline executable in both positions to measure host variation. Do not build or run other owned test workloads concurrently with timing.
Reproduce the native macOS capture matrix
Use a separate copy of the standalone fixture above. Add this dependency under
[dependencies]in Cargo.toml, with the prepared GPUI path:Save the following as
src/native.rs. It is the native capture driver used in review. The unusedpanelargument in its scroll helper is historical fixture code; scroll position comes from the dispatched event.Change the benchmark entry point to retain it as an unused function, then call the native driver instead. This adapts the CPU fixture's entry point for the native matrix; it does not modify production menu code.
After generating the fixture lockfile, build without/with
--features candidateand preserve both executables before the next build replaces the output. Run each with a freshMENU_NATIVE_OUTPUTdirectory on macOS. Do not enable the optionalgeometryfeature here. Compare decoded PNG RGBA bytes, not compressed PNG sizes. Repeat both unchanged binaries to distinguish deterministic candidate differences from unstable rendering. The 1,236-bound comparison was a separate passive Element-wrapper probe, and is not performed by this uninstrumented image driver.Measured artifact fingerprints
de1425ea4177ab97be69012ee68bb498ddef4c18ccb16945b24965044ea048527010fe819b95bdfb67f128a5f69863210238cb15cf412c46abb2fa69f2003b2f604d7dc9c674dbb6e75c2c5fb2ee3d6a12861873ea6390f01898acb18e4c3da43751b37796055d12e3b955b9afb117878d82ba26d35629afd460d7b070885c17756840ffdcd1e6c02aa9a34a96845e0d16ddada610a192b5ebe817ff0da4d8782242767959072ca7892ce82e75f4f378f799924fad2477d0deb33fa457a901cfa7b62e9181eb39f64da77771a79f17f0e4e72dffffc67107b649f2315d62909d592e04661fa6b0f1ab9d6aa4e17c46294af0c476966cc06c48dfba3f2c6d7b5f149bb65183a42b4335fd19c6fd92605aa8503959ddcb00ffcb03a528ba4f65e8Executable and lock hashes identify the measured artifacts; they are not expected to match across platforms or independently resolved lockfiles. Original archive results from Linux were checked for arithmetic consistency but are not used as locally reproduced Linux evidence.
Merge danger
This is reversible by reverting one commit. It changes the shared dropdown component used by editor, settings and screenshot controls. The main risks are intrinsic-width changes, scroll boundaries and pending-click lifetimes; the eager fallbacks and geometry/interaction/native comparisons address those paths. First-open width measurement and exact-label cache validation still scale with item count, and holding the left button intentionally gives up virtualization until release. No migrations, persisted-data changes, feature flags or new production dependencies.
The PR appears safe to merge; no actionable issues were found.Summary
Reduces work when drawing overflowing, fixed-height menus.
No actionable issues were found. Tests and native rendering were not run during this review.
Reviews (1) · Last reviewed commit: "perf(desktop-gpui): virtualize long drop..." · Reviewed by Greptile