full_integration_plan.md

  1# GPUI Scheduler Integration
  2
  3This document describes the integration of GPUI's async execution with the scheduler crate, including architecture, design decisions, and lessons learned.
  4
  5## Goal
  6
  7Unify GPUI's async execution with the scheduler crate, eliminating duplicate blocking/scheduling logic and enabling deterministic testing.
  8
  9---
 10
 11## Architecture
 12
 13```
 14┌──────────────────────────────────────────────────────────────────┐
 15│                            GPUI                                   │
 16│                                                                   │
 17│  ┌────────────────────────┐    ┌──────────────────────────────┐  │
 18│  │  gpui::Background-     │    │  gpui::ForegroundExecutor    │  │
 19│  │  Executor              │    │   - inner: scheduler::       │  │
 20│  │   - scheduler: Arc<    │    │           ForegroundExecutor │  │
 21│  │       dyn Scheduler>   │    │   - dispatcher: Arc          │  │
 22│  │   - dispatcher: Arc    │    └──────────────┬───────────────┘  │
 23│  └───────────┬────────────┘                   │                   │
 24│              │                                │                   │
 25│              │  (creates temporary            │ (wraps)           │
 26│              │   scheduler::Background-       │                   │
 27│              │   Executor when spawning)      │                   │
 28│              │                                │                   │
 29│              │    ┌───────────────────────────┘                   │
 30│              │    │                                               │
 31│              ▼    ▼                                               │
 32│  ┌─────────────────────────────────────────────────────────────┐ │
 33│  │                    Arc<dyn Scheduler>                        │ │
 34│  └──────────────────────────┬──────────────────────────────────┘ │
 35│                             │                                     │
 36│            ┌────────────────┴────────────────┐                   │
 37│            │                                 │                    │
 38│            ▼                                 ▼                    │
 39│  ┌───────────────────────┐     ┌───────────────────────────┐    │
 40│  │   PlatformScheduler   │     │      TestScheduler        │    │
 41│  │   (production)        │     │   (deterministic tests)   │    │
 42│  └───────────────────────┘     └───────────────────────────┘    │
 43└──────────────────────────────────────────────────────────────────┘
 44```
 45
 46### Scheduler Trait
 47
 48The scheduler crate provides:
 49- `Scheduler` trait with `block()`, `schedule_foreground()`, `schedule_background_with_priority()`, `timer()`, `clock()`
 50- `TestScheduler` implementation for deterministic testing
 51- `ForegroundExecutor` and `BackgroundExecutor` that wrap `Arc<dyn Scheduler>`
 52- `Task<T>` type with `ready()`, `is_ready()`, `detach()`, `from_async_task()`
 53
 54### PlatformScheduler
 55
 56`PlatformScheduler` in GPUI (`crates/gpui/src/platform_scheduler.rs`):
 57- Implements `Scheduler` trait for production use
 58- Wraps `PlatformDispatcher` (Mac, Linux, Windows)
 59- Uses `parking::Parker` for blocking operations
 60- Uses `dispatch_after` for timers
 61- Provides a `PlatformClock` that delegates to the dispatcher
 62
 63### GPUI Executors
 64
 65GPUI's executors (`crates/gpui/src/executor.rs`):
 66- `gpui::ForegroundExecutor` wraps `scheduler::ForegroundExecutor` internally
 67- `gpui::BackgroundExecutor` holds `Arc<dyn Scheduler>` directly
 68- Select `TestScheduler` or `PlatformScheduler` based on dispatcher type
 69- Wrap `scheduler::Task<T>` in a thin `gpui::Task<T>` that adds `detach_and_log_err()`
 70- Use `Scheduler::block()` for all blocking operations
 71
 72---
 73
 74## Design Decisions
 75
 76### Key Design Principles
 77
 781. **No optional fields**: Both test and production paths use the same executor types with different `Scheduler` implementations underneath.
 79
 802. **Scheduler owns blocking logic**: The `Scheduler::block()` method handles all blocking, including timeout and task stepping (for tests).
 81
 823. **GPUI Task wrapper**: Thin wrapper around `scheduler::Task` that adds `detach_and_log_err()` which requires `&App`.
 83
 84### Foreground Priority Not Supported
 85
 86`ForegroundExecutor::spawn_with_priority` accepts a priority parameter but ignores it. This is acceptable because:
 87- macOS (primary platform) ignores foreground priority anyway
 88- TestScheduler runs foreground tasks in order
 89- There are no external callers of this method in the codebase
 90
 91### Session IDs for Foreground Isolation
 92
 93Each `ForegroundExecutor` gets a `SessionId` to prevent reentrancy when blocking. This ensures that when blocking on a future, we don't run foreground tasks from the same session.
 94
 95### Runtime Scheduler Selection
 96
 97In test builds, we check `dispatcher.as_test()` to choose between `TestScheduler` and `PlatformScheduler`. This allows the same executor types to work in both test and production environments.
 98
 99### Profiler Integration
100
101The profiler task timing infrastructure continues to work because:
102- `PlatformScheduler::schedule_background_with_priority` calls `dispatcher.dispatch()`
103- `PlatformScheduler::schedule_foreground` calls `dispatcher.dispatch_on_main_thread()`
104- All platform dispatchers wrap task execution with profiler timing
105
106---
107
108## Intentional Removals
109
110### `spawn_labeled` and `deprioritize`
111
112**What was removed**:
113- `BackgroundExecutor::spawn_labeled(label: TaskLabel, future)`
114- `BackgroundExecutor::deprioritize(label: TaskLabel)`
115- `TaskLabel` type
116
117**Why**: These were only used in a few places for test ordering control. The new priority-weighted scheduling in `TestScheduler` provides similar functionality through `Priority::High/Medium/Low`.
118
119**Migration**: Use `spawn()` instead of `spawn_labeled()`. For test ordering, use explicit synchronization (channels, etc.) or priority levels.
120
121### `start_waiting` / `finish_waiting` Debug Methods
122
123**What was removed**:
124- `BackgroundExecutor::start_waiting()`
125- `BackgroundExecutor::finish_waiting()`
126- Associated `waiting_backtrace` tracking in TestDispatcher
127
128**Why**: The new `TracingWaker` in `TestScheduler` provides better debugging capability. Run tests with `PENDING_TRACES=1` to see backtraces of all pending futures when parking is forbidden.
129
130### Realtime Priority
131
132**What was removed**: `Priority::Realtime` variant and associated OS thread spawning.
133
134**Why**: There were no in-tree call sites using realtime priority. The correctness/backpressure semantics are non-trivial:
135- Blocking enqueue risks stalling latency-sensitive threads
136- Non-blocking enqueue implies dropping runnables under load, which breaks correctness for general futures
137
138Rather than ship ambiguous or risky semantics, we removed the API until there is a concrete in-tree use case.
139
140---
141
142## Lessons Learned
143
144These lessons were discovered during integration testing and represent important design constraints.
145
146### 1. Never Cache `Entity<T>` in Process-Wide Statics
147
148**Problem**: `gpui::Entity<T>` is a handle tied to a particular `App`'s entity-map. Storing an `Entity<T>` in a process-wide static (`OnceLock`, `LazyLock`, etc.) and reusing it across different `App` instances causes:
149- "used a entity with the wrong context" panics
150- `Option::unwrap()` failures in leak-detection clone paths
151- Nondeterministic behavior depending on test ordering
152
153**Solution**: Cache plain data (env var name, URL, etc.) in statics, and create `Entity<T>` per-`App`.
154
155**Guideline**: Never store `gpui::Entity<T>` or other `App`-context-bound handles in process-wide statics unless explicitly keyed by `App` identity.
156
157### 2. `block_with_timeout` Behavior Depends on Tick Budget
158
159**Problem**: In `TestScheduler`, "timeout" behavior depends on an internal tick budget (`timeout_ticks`), not just elapsed wall-clock time. During the allotted ticks, the scheduler can poll futures and step other tasks.
160
161**Implications**:
162- A future can complete "within a timeout" in tests due to scheduler progress, even without explicit `advance_clock()`
163- Yielding does not advance time
164- If a test needs time to advance, it must do so explicitly via `advance_clock()`
165
166**For deterministic timeout tests**: Set `scheduler.set_timeout_ticks(0..=0)` to prevent any scheduler stepping during timeout, then explicitly advance time.
167
168### 3. Realtime Priority Must Panic in Tests
169
170**Problem**: `Priority::Realtime` spawns dedicated OS threads outside the test scheduler, which breaks determinism and causes hangs/flakes.
171
172**Solution**: The test dispatcher's `spawn_realtime` implementation panics with a clear message. This is an enforced invariant, not an implementation detail.
173
174---
175
176## Test Helpers
177
178Test-only methods on `BackgroundExecutor`:
179- `block_test()` - for running async tests synchronously
180- `advance_clock()` - move simulated time forward
181- `tick()` - run one task
182- `run_until_parked()` - run all ready tasks
183- `allow_parking()` / `forbid_parking()` - control parking behavior
184- `simulate_random_delay()` - yield randomly for fuzzing
185- `rng()` - access seeded RNG
186- `set_block_on_ticks()` - configure timeout tick range for block operations
187
188---
189
190## Code Quality Notes
191
192### `dispatch_after` Panics in TestDispatcher
193
194This is intentional:
195```rust
196fn dispatch_after(&self, _duration: Duration, _runnable: RunnableVariant) {
197    panic!(
198        "dispatch_after should not be called in tests. \
199        Use BackgroundExecutor::timer() which uses the scheduler's native timer."
200    );
201}
202```
203
204In tests, `TestScheduler::timer()` creates native timers without using `dispatch_after`. Any code hitting this panic has a bug.
205
206---
207
208## Files Changed
209
210Key files modified during integration:
211
212- `crates/scheduler/src/scheduler.rs` - `Scheduler::block()` signature takes `Pin<&mut dyn Future>` and returns `bool`
213- `crates/scheduler/src/executor.rs` - Added `from_async_task()`
214- `crates/scheduler/src/test_scheduler.rs` - Deterministic scheduling implementation
215- `crates/gpui/src/executor.rs` - Rewritten to use scheduler executors
216- `crates/gpui/src/platform_scheduler.rs` - New file implementing `Scheduler` for production
217- `crates/gpui/src/platform/test/dispatcher.rs` - Simplified to delegate to TestScheduler
218- `crates/gpui/src/platform.rs` - Simplified `RunnableVariant`, removed `TaskLabel`
219- Platform dispatchers (mac/linux/windows) - Removed label parameter from dispatch
220
221---
222
223## Future Considerations
224
2251. **Foreground priority support**: If needed, add `schedule_foreground_with_priority` to the `Scheduler` trait.
226
2272. **Profiler integration in scheduler**: Could move task timing into the scheduler crate for more consistent profiling.
228
2293. **Additional test utilities**: The `TestScheduler` could be extended with more debugging/introspection capabilities.