refactor: replace AppProxy poll loop with MainThreadPoster#11
Closed
BumpyClock wants to merge 1 commit into
Closed
refactor: replace AppProxy poll loop with MainThreadPoster#11BumpyClock wants to merge 1 commit into
BumpyClock wants to merge 1 commit into
Conversation
AppProxy scheduled cross-thread work onto the main thread by draining an mpsc channel from a foreground task that woke every 16ms via a background timer. That poll loop was flagged as a wart: an idle tray app paid a fixed wakeup cost forever, purely because the fork had no dependency-free way to wake the main thread from a Send context. The fork now provides that primitive. App::main_thread_poster hands back a Clone+Send+Sync MainThreadPoster whose post() enqueues a closure and wakes the run loop through the platform dispatcher, so an idle app stays parked. Back AppProxy with the poster and delete the timer loop. The shutdown boundary stays ours: a `closed` AtomicBool, set at ShutdownRequested (which precedes app teardown), is checked in dispatch and re-checked inside each posted closure on the main thread. Since close() also runs on the main thread, any closure the pump dispatches after close() drops its payload, preserving the old drain loop's discard-after-close contract. The poster's own after-teardown false is a second layer, not a replacement. Pins gpui/gpui_platform and the vendor submodule to 2ee4fa6 (gpui#7). The workspace version and COMPATIBILITY.md bump is deferred to merge time via /update-gpui discipline.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
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.
Motivation
AppProxyscheduled cross-thread work onto the main thread by draining anstd::mpscchannel from a foreground task that woke every 16ms via abackground timer. The loop was flagged as a wart in the app-platform work: an
idle tray app pays a fixed wakeup cost forever, purely because the fork had no
dependency-free way to wake the main thread from a
Sendcontext. That isexactly the battery profile a tray-first app is supposed to avoid.
The swap
The fork now exposes the primitive (gpui#7):
App::main_thread_poster()handsback a
Clone + Send + SyncMainThreadPosterwhosepost()enqueues aclosure and wakes the run loop through the platform dispatcher — no polling, an
idle app stays parked.
AppProxyis now backed by the poster; the timer/drain loop and itsgpui::Task(_drain_task) are deleted.closed: AtomicBool, set atShutdownRequested(which precedes app teardown), is checked indispatchand re-checked inside every posted closure on the main thread. Because
close()also runs on the main thread, any closure the pump dispatches afterclose()drops its payload — preserving the old drain loop'sdiscard-after-close contract. The poster's own after-teardown
falseis asecond layer, not a replacement.
with three
#[gpui::test]behavior tests (runs-until-closed,queued-work-discarded-after-close, callback-shutdown-discards-following-work)
driving a real
&mut App. Addsgpuitest-supportto the crate'sdev-dependencies (already used by
crates/ui).Pinned SHA
gpui / gpui_platform
revand thevendor/gpuisubmodule are pinned to2ee4fa6587f9fc8eef012f86897034cb7d859d9d(main + 1, additive). The workspaceversion and
COMPATIBILITY.mdbump are intentionally not done here — thathappens at merge time via
/update-gpuidiscipline.Merge preconditions
via
/update-gpui.feat/app-platform.Gates
cargo test -p gpui-component-app— 49 unit + headless + doc, all greencargo clippy --workspace --all-targets -- --deny warnings— cleancargo fmt --check— cleanapp_shell_tray --smokeandapp_shell --smoke— both exit 0