mixa/zed
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Remove unused execution tracking from TestScheduler and TestDispatcher

Browse Source 
The execution_hash/execution_count tracking was added speculatively but
has no actual consumers. Removing to reduce complexity.
This commit is contained in:
Nathan Sobo
2025-12-14 05:40:03 -07:00
parent 52e2650270
commit c39581d52e
4 changed files with 19 additions and 230 deletions
Download Patch File Download Diff File Expand all files Collapse all files

53
crates/gpui/src/platform/test/dispatcher.rs
View File

@@ -7,7 +7,7 @@ use rand::prelude::*;
use scheduler::{Clock, TestScheduler, TestSchedulerConfig};
use std::{
future::Future,
hash::{Hash, Hasher},
hash::Hash,
ops::RangeInclusive,
pin::Pin,
sync::{
@@ -90,13 +90,6 @@ struct TestDispatcherState {
deprioritized_task_labels: HashSet<TaskLabel>,
block_on_ticks: RangeInclusive<usize>,
unparkers: Vec<Unparker>,
/// Tracks execution order for determinism verification.
/// This hash is updated each time a task is executed, incorporating
/// the task's source location. If two runs with the same seed produce
/// different hashes, there is non-determinism in the test.
execution_hash: u64,
/// Count of tasks executed, for debugging.
execution_count: u64,
}
impl TestDispatcher {
@@ -122,8 +115,6 @@ impl TestDispatcher {
deprioritized_task_labels: Default::default(),
block_on_ticks: 0..=1000,
unparkers: Default::default(),
execution_hash: 0,
execution_count: 0,
};
TestDispatcher {
@@ -297,7 +288,6 @@ impl TestDispatcher {
drop(state);
// Log task location before running (helps identify which task is executing)
// Also update execution hash for determinism tracking
let location_str = match &runnable {
RunnableVariant::Meta(r) => {
let loc = r.metadata().location;
@@ -306,24 +296,11 @@ impl TestDispatcher {
RunnableVariant::Compat(_) => "compat-task".to_string(),
};
// Update execution hash with task location for determinism verification
{
let mut state = self.state.lock();
let mut hasher = std::collections::hash_map::DefaultHasher::new();
state.execution_hash.hash(&mut hasher);
location_str.hash(&mut hasher);
state.execution_count.hash(&mut hasher);
state.execution_hash = hasher.finish();
state.execution_count += 1;
}
dispatcher_log!(
"tick() RUNNING {} task from {} | main_thread={} | exec_count={} exec_hash={:016x}",
"tick() RUNNING {} task from {} | main_thread={}",
task_source,
location_str,
main_thread,
self.state.lock().execution_count,
self.state.lock().execution_hash
);
match runnable {
@@ -427,32 +404,6 @@ impl TestDispatcher {
/// seed produce different execution hashes at the same point, there is
/// non-determinism in the execution (likely from real OS threads, smol::spawn,
/// or other sources outside TestDispatcher's control).
///
/// # Example
/// ```ignore
/// let hash_before = dispatcher.execution_hash();
/// // ... do some work ...
/// let hash_after = dispatcher.execution_hash();
/// eprintln!("Execution hash: {} -> {} (count: {})",
/// hash_before, hash_after, dispatcher.execution_count());
/// ```
pub fn execution_hash(&self) -> u64 {
self.state.lock().execution_hash
}
/// Returns the number of tasks executed so far.
pub fn execution_count(&self) -> u64 {
self.state.lock().execution_count
}
/// Resets the execution hash and count. Useful for isolating
/// determinism checks to specific sections of a test.
pub fn reset_execution_tracking(&self) {
let mut state = self.state.lock();
state.execution_hash = 0;
state.execution_count = 0;
}
/// Returns a reference to the underlying TestScheduler.
/// This can be used for advanced testing scenarios that need direct scheduler access.
pub fn scheduler(&self) -> &Arc<TestScheduler> {

143
crates/scheduler/full_integration_plan.md
View File

@@ -9,12 +9,7 @@
- Updated all platform dispatchers (Mac, Linux, Windows, Test) to remove `Scheduler` variant handling
- Updated `PlatformScheduler` to use `RunnableVariant::Meta` instead of `RunnableVariant::Scheduler`
### 2. Added Execution Tracking to TestScheduler (Phase 3c) - DONE
- Added `execution_hash: u64` and `execution_count: u64` to `SchedulerState`
- Added `execution_hash()`, `execution_count()`, `reset_execution_tracking()` methods
- Execution tracking updates in `step()` after task selection, hashing source location
### 3. Waiting Hints (Phase 3b) - SKIPPED
### 2. Waiting Hints (Phase 3b) - SKIPPED
- NOT NEEDED: TestScheduler already has superior `pending_traces` system with `TracingWaker`
- Automatically captures backtraces for ALL pending futures via `PENDING_TRACES=1` env var
- More comprehensive than manual waiting hints
@@ -26,11 +21,9 @@
- Add `is_ready()` method to scheduler's `Task<T>` if needed
- Keep `detach_and_log_err` as extension trait in GPUI (needs `App` context)
2. **Update TestDispatcher to delegate execution tracking** - Wire TestDispatcher's `execution_hash()` and `execution_count()` to call TestScheduler's new methods (currently TestDispatcher has its own tracking)
### Medium-term:
3. **Eliminate RunnableVariant entirely (Phase 2a full)** - Remove `Compat` variant by converting timer callbacks to use `RunnableMeta`
4. **Delegate task queues to TestScheduler (Phase 2b)** - Most complex change
2. **Eliminate RunnableVariant entirely (Phase 2a full)** - Remove `Compat` variant by converting timer callbacks to use `RunnableMeta`
3. **Delegate task queues to TestScheduler (Phase 2b)** - Most complex change
---
@@ -219,22 +212,7 @@ Some GPUI features should move to the scheduler crate to reduce duplication.
**Problem**: When tests hang, GPUI captures waiting hints and backtraces for debugging.
**Solution**: Add optional debug info to scheduler's parking mechanism.
**Steps**:
1. Add `set_waiting_hint(&str)` to `TestScheduler`
2. Add backtrace capture on park timeout
3. Wire through from GPUI's `BackgroundExecutor::set_waiting_hint`
#### 3c: Execution Tracking
**Problem**: `TestDispatcher` tracks execution hash/count for determinism verification.
**Solution**: Add to `TestScheduler`.
**Steps**:
1. Add `execution_hash()` and `execution_count()` to `TestScheduler`
2. Update on each task execution with source location hash
**Solution**: NOT NEEDED - TestScheduler already has superior `pending_traces` system with `TracingWaker` that automatically captures backtraces for ALL pending futures via `PENDING_TRACES=1` env var.
---
@@ -320,125 +298,22 @@ pub enum RunnableVariant {
- Convert to `RunnableVariant::Meta`
- Update all dispatcher match arms
#### 2. Add execution tracking to TestScheduler (Phase 3c)
#### 2. Eliminate RunnableVariant::Compat (Phase 2a full)
Add to `TestScheduler` for determinism verification:
```rust
// scheduler/src/test_scheduler.rs
// Add to SchedulerState:
struct SchedulerState {
// ... existing fields ...
execution_hash: u64,
execution_count: u64,
}
impl TestScheduler {
pub fn execution_hash(&self) -> u64 {
self.state.lock().execution_hash
}
pub fn execution_count(&self) -> u64 {
self.state.lock().execution_count
}
pub fn reset_execution_tracking(&self) {
let mut state = self.state.lock();
state.execution_hash = 0;
state.execution_count = 0;
}
}
// In step(), after selecting runnable:
fn step(&self) -> bool {
// ... select runnable ...
// Update execution tracking
let mut state = self.state.lock();
let mut hasher = DefaultHasher::new();
state.execution_hash.hash(&mut hasher);
runnable.metadata().location.file().hash(&mut hasher);
runnable.metadata().location.line().hash(&mut hasher);
state.execution_count.hash(&mut hasher);
state.execution_hash = hasher.finish();
state.execution_count += 1;
drop(state);
runnable.run();
true
}
```
Then update `TestDispatcher` to delegate:
```rust
// gpui/src/platform/test/dispatcher.rs
impl TestDispatcher {
pub fn execution_hash(&self) -> u64 {
self.scheduler.execution_hash()
}
pub fn execution_count(&self) -> u64 {
self.scheduler.execution_count()
}
}
```
#### 3. Add waiting hints to TestScheduler (Phase 3b)
```rust
// scheduler/src/test_scheduler.rs
// Add to SchedulerState:
struct SchedulerState {
// ... existing fields ...
waiting_hint: Option<String>,
waiting_backtrace: Option<Backtrace>,
}
impl TestScheduler {
pub fn set_waiting_hint(&self, hint: impl Into<String>) {
self.state.lock().waiting_hint = Some(hint.into());
}
pub fn clear_waiting_hint(&self) {
let mut state = self.state.lock();
state.waiting_hint = None;
state.waiting_backtrace = None;
}
pub fn waiting_hint(&self) -> Option<String> {
self.state.lock().waiting_hint.clone()
}
}
// In park(), include hint in panic message:
fn park(&self, deadline: Option<Instant>) -> bool {
let state = self.state.lock();
if !state.allow_parking {
let hint = state.waiting_hint.as_deref().unwrap_or("no hint");
panic!("test parked unexpectedly. hint: {}", hint);
}
// ... rest of parking logic ...
}
```
Convert timer callbacks to use `RunnableMeta` instead of plain `Runnable`, eliminating the need for the `Compat` variant entirely.
### Medium-term (Higher Effort)
4. **Eliminate RunnableVariant** (Phase 2a full)
- Requires coordinated changes across all dispatchers
- Significant simplification
5. **Unify Task types** (Phase 1)
3. **Unify Task types** (Phase 1)
- After RunnableMeta unified, this becomes cleaner
### Long-term (Optional)
6. **Delegate task queues** (Phase 2b)
4. **Delegate task queues** (Phase 2b)
- Most complex change
- Only if clear benefit emerges
7. **Executor unification** (Phase 4)
5. **Executor unification** (Phase 4)
- Defer unless concrete need arises
## Technical Considerations

11
crates/scheduler/plan.md
View File

@@ -46,15 +46,20 @@ This document outlines the integration of the `scheduler` crate into GPUI, provi
### Phase 2: GPUI Adapters ✅
- Created `PlatformScheduler` adapter implementing `Scheduler` trait
- Added `RunnableVariant::Scheduler` for scheduler crate's runnables
- Updated all platform dispatchers (Mac, Linux, Windows, Test)
### Phase 2a: Unified RunnableMeta ✅
- Re-exported `RunnableMeta` from scheduler crate (removed GPUI's duplicate definition)
- Removed `RunnableVariant::Scheduler` variant (now uses `RunnableVariant::Meta` for both)
- Simplified all dispatcher match arms
### Phase 3: Hybrid TestDispatcher Integration ✅
`TestDispatcher` uses `TestScheduler` internally for timing/clock/rng while keeping its own task queues for `RunnableVariant` handling.
**Why hybrid approach:**
- GPUI uses `RunnableVariant` with 3 variants (Meta, Compat, Scheduler)
- GPUI uses `RunnableVariant` with 2 variants (Meta, Compat)
- `TestScheduler` expects `Runnable<RunnableMeta>` only
- Hybrid allows unified timing without breaking existing behavior
@@ -124,7 +129,7 @@ Full executor composition (GPUI executors wrapping scheduler executors) is **def
### GPUI Crate
- `src/platform/platform_scheduler.rs` - New `PlatformScheduler` adapter
- `src/platform.rs` - `RunnableVariant::Scheduler`, platform_scheduler module
- `src/platform.rs` - Re-exports `RunnableMeta` from scheduler, platform_scheduler module
- `src/platform/test/dispatcher.rs` - `TestScheduler` integration, `new(seed: u64)`
- `src/platform/mac/dispatcher.rs` - `trampoline_scheduler`, handle new variant
- `src/platform/linux/dispatcher.rs` - Handle new variant

42
crates/scheduler/src/test_scheduler.rs
View File

@@ -10,11 +10,9 @@ use rand::prelude::*;
use std::{
any::type_name_of_val,
collections::{BTreeMap, HashSet, VecDeque},
collections::hash_map::DefaultHasher,
env,
fmt::Write,
future::Future,
hash::{Hash, Hasher},
mem,
ops::RangeInclusive,
panic::{self, AssertUnwindSafe},
@@ -93,8 +91,6 @@ impl TestScheduler {
capture_pending_traces: config.capture_pending_traces,
pending_traces: BTreeMap::new(),
next_trace_id: TraceId(0),
execution_hash: 0,
execution_count: 0,
})),
clock: Arc::new(TestClock::new()),
thread: thread::current(),
@@ -125,25 +121,6 @@ impl TestScheduler {
self.state.lock().allow_parking
}
/// Returns the current execution hash for determinism verification.
/// If two runs with the same seed produce different hashes, there is
/// non-determinism in the test.
pub fn execution_hash(&self) -> u64 {
self.state.lock().execution_hash
}
/// Returns the number of tasks that have been executed.
pub fn execution_count(&self) -> u64 {
self.state.lock().execution_count
}
/// Resets execution tracking state (hash and count).
pub fn reset_execution_tracking(&self) {
let mut state = self.state.lock();
state.execution_hash = 0;
state.execution_count = 0;
}
/// Create a foreground executor for this scheduler
pub fn foreground(self: &Arc<Self>) -> ForegroundExecutor {
let session_id = {
@@ -255,19 +232,6 @@ impl TestScheduler {
};
if let Some(runnable) = runnable {
// Update execution tracking with task location for determinism verification
{
let location = runnable.runnable.metadata().location;
let mut state = self.state.lock();
let mut hasher = DefaultHasher::new();
state.execution_hash.hash(&mut hasher);
location.file().hash(&mut hasher);
location.line().hash(&mut hasher);
state.execution_count.hash(&mut hasher);
state.execution_hash = hasher.finish();
state.execution_count += 1;
}
runnable.run();
return true;
}
@@ -519,12 +483,6 @@ struct SchedulerState {
capture_pending_traces: bool,
next_trace_id: TraceId,
pending_traces: BTreeMap<TraceId, Backtrace>,
/// Tracks execution order for determinism verification.
/// This hash is updated each time a task is executed, incorporating
/// the task's source location.
execution_hash: u64,
/// Count of tasks executed, for debugging.
execution_count: u64,
}
const WAKER_VTABLE: RawWakerVTable = RawWakerVTable::new(