diff --git a/thoughts/2025-08-31_15-47-17_unified-scheduler-architecture.md b/thoughts/2025-08-31_15-47-17_unified-scheduler-architecture.md index d3317bf9291955f2572a63a2b86816d22d675fe1..694ecca5d3c92b2a6fe80256b56ece960c402576 100644 --- a/thoughts/2025-08-31_15-47-17_unified-scheduler-architecture.md +++ b/thoughts/2025-08-31_15-47-17_unified-scheduler-architecture.md @@ -20,17 +20,19 @@ Key design principles: │ Shared Crate │ ├─────────────────────────────────────────┤ │ Scheduler trait: │ -│ - spawn() - core Send futures │ -│ - spawn_foreground() - non-Send │ +│ - Core object-safe interface │ │ - Platform integration (park, now) │ │ │ │ TestScheduler: │ -│ - Implements Scheduler + test features │ +│ - Should implement Scheduler + test features │ │ - deprioritize() - test-only method │ │ - spawn_labeled() - labels for testing │ +│ - Task lifecycle tracking │ │ │ -│ GcdScheduler/ThreadPoolScheduler: │ -│ - Minimal Scheduler implementations │ +│ Generic spawn helpers: │ +│ - spawn() / spawn_foreground() │ +│ - timer(), block(), block_with_timeout()│ +│ - Future-based API for trait objects │ └─────────────────────────────────────────┘ ▲ ┌─────────┼─────────┐ @@ -43,6 +45,21 @@ Key design principles: └──────────────┘ └─────────────────┘ ``` +## Platform Scheduler Implementations + +Platform-specific scheduler implementations **should remain in GPUI's platform modules**: + +- **MacScheduler**: Should implement `Scheduler` trait, uses GCD APIs +- **LinuxScheduler**: Should implement `Scheduler` trait, uses thread pools + calloop +- **WindowsScheduler**: Should implement `Scheduler` trait, uses Windows ThreadPool APIs + +**Rationale**: Platform implementations are substantial and deeply integrated with: +- Platform-specific threading APIs (GCD, Windows ThreadPool, etc.) +- GPUI's event loop integration (main thread messaging) +- Platform-specific performance optimizations + +The shared crate provides only the trait definition and generic helpers, while platform-specific dispatchers implement the `Scheduler` trait directly in GPUI. + ## Scheduler Trait Definition ```rust @@ -525,48 +542,93 @@ impl TestScheduler { - `is_task_running()` provides task status for Cloud session validation - Test-specific state stays in TestScheduler -## Production Schedulers +## Platform Scheduler Implementations in GPUI + +The following example shows how existing GPUI platform dispatchers would implement the `Scheduler` trait. These implementations **remain in GPUI's platform modules**, not in the shared scheduler crate. ```rust -pub struct GcdScheduler { - main_queue: dispatch_queue_t, - background_queue: dispatch_queue_t, -} +// crates/gpui/src/platform/mac/scheduler.rs (renamed from dispatcher.rs) +// This should implement Scheduler directly on existing platform-specific code -impl Scheduler for GcdScheduler { +// Example implementation (to be added in Phase 1): +impl Scheduler for MacDispatcher { fn schedule(&self, runnable: Runnable) { + // Direct mapping to existing GCD implementation unsafe { - dispatch_async_f(self.background_queue, runnable.into_raw().as_ptr() as *mut c_void, Some(trampoline)); + dispatch_async_f( + dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_HIGH.try_into().unwrap(), 0), + runnable.into_raw().as_ptr() as *mut c_void, + Some(trampoline), + ); } } fn schedule_labeled(&self, runnable: Runnable, _label: TaskLabel) { - // Production scheduler ignores labels + // Production scheduler ignores labels (existing behavior) unsafe { - dispatch_async_f(self.background_queue, runnable.into_raw().as_ptr() as *mut c_void, Some(trampoline)); + dispatch_async_f( + dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_HIGH.try_into().unwrap(), 0), + runnable.into_raw().as_ptr() as *mut c_void, + Some(trampoline), + ); } } fn schedule_foreground(&self, runnable: Runnable) { + // Existing dispatch_on_main_thread implementation + unsafe { + dispatch_async_f( + dispatch_get_main_queue(), + runnable.into_raw().as_ptr() as *mut c_void, + Some(trampoline), + ); + } + } + + fn schedule_after(&self, duration: Duration, runnable: Runnable) { + // Existing dispatch_after implementation unsafe { - dispatch_async_f(self.main_queue, runnable.into_raw().as_ptr() as *mut c_void, Some(trampoline)); + let queue = dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_HIGH.try_into().unwrap(), 0); + let when = dispatch_time(DISPATCH_TIME_NOW as u64, duration.as_nanos() as i64); + dispatch_after_f(when, queue, runnable.into_raw().as_ptr() as *mut c_void, Some(trampoline)); } } fn is_main_thread(&self) -> bool { - // macOS-specific main thread detection - unsafe { msg_send![class!(NSThread), isMainThread] } + // Existing macOS-specific main thread detection + unsafe { + let is_main_thread: BOOL = msg_send![class!(NSThread), isMainThread]; + is_main_thread == YES + } + } + + fn now(&self) -> Instant { + Instant::now() // Existing implementation + } + + fn park(&self, timeout: Option) -> bool { + // Existing parking implementation + if let Some(timeout) = timeout { + self.parker.lock().park_timeout(timeout) + } else { + self.parker.lock().park(); + true + } } - fn now(&self) -> Instant { Instant::now() } + fn unparker(&self) -> Unparker { + // Existing unparker implementation + self.parker.lock().unparker() + } } ``` -**Explanation:** -- Production schedulers implement object-safe `Scheduler` trait -- No test-specific features or task state tracking -- Minimal implementation with direct dispatch to GCD queues -- Test features only available via `TestScheduler` wrapper in GPUI +**Key Points:** +- Platform scheduler implementations **remain in GPUI platform modules** (e.g., `platform/mac/`, `platform/linux/`, `platform/windows/`) +- Existing platform dispatchers implement `Scheduler` trait directly - no code needs to be moved +- Substantial platform-specific code stays where it belongs, integrated with event loops +- GPUI executors use trait objects: `Arc` pointing to platform implementations +- Shared crate provides only trait definition + TestScheduler + generic helpers ## GPUI Integration @@ -857,7 +919,10 @@ impl CloudSimulatedScheduler { ### Phase 1: Core Infrastructure 1. Define `Scheduler` trait (core methods only) 2. Implement `TestScheduler` (with test features like `deprioritize()`) -3. Implement production schedulers (GCD, ThreadPool) +3. Make existing GPUI platform dispatchers implement `Scheduler` trait + - MacDispatcher should implement `Scheduler` for GCD integration + - LinuxDispatcher should implement `Scheduler` for thread pools + - WindowsDispatcher should implement `Scheduler` for Windows ThreadPool 4. Define `Task` with mandatory TaskId ### Phase 2: GPUI Migration @@ -971,16 +1036,16 @@ This design keeps test concerns in TestScheduler while maintaining production sa ### Platform Backend Files **macOS:** -- `crates/gpui/src/platform/mac/dispatcher.rs` - `MacDispatcher` → `GcdScheduler` +- `crates/gpui/src/platform/mac/dispatcher.rs` - `MacDispatcher` should implement `Scheduler` trait **Linux:** -- `crates/gpui/src/platform/linux/dispatcher.rs` - `LinuxDispatcher` → `ThreadPoolScheduler` +- `crates/gpui/src/platform/linux/dispatcher.rs` - `LinuxDispatcher` should implement `Scheduler` trait **Windows:** -- `crates/gpui/src/platform/windows/dispatcher.rs` - `WindowsDispatcher` → `ThreadPoolScheduler` +- `crates/gpui/src/platform/windows/dispatcher.rs` - `WindowsDispatcher` should implement `Scheduler` trait **Test:** -- `crates/gpui/src/platform/test/dispatcher.rs` - `TestDispatcher` → `TestScheduler` +- `crates/gpui/src/platform/test/dispatcher.rs` - `TestDispatcher` → `TestScheduler` (moved to shared crate) ## Compatibility Checklist @@ -1058,6 +1123,6 @@ GPUI BackgroundExecutor/ForegroundExecutor can **directly swap** `PlatformDispat 2. **Implement TestScheduler** with task tracking and test features 3. **Update GPUI executors** to use trait objects 4. **Create Cloud wrapper** with session coordination -5. **Migrate platform backends** to new scheduler implementations +5. **Make platform dispatchers implement** `Scheduler` trait (add trait impl to existing dispatchers) 6. **Update tests** to use new architecture 7. **Validate performance** and backward compatibility