From e7a9988d1abfedf67ecf5ae03ce15ecd0b9d82a1 Mon Sep 17 00:00:00 2001 From: iceweasel-oai Date: Mon, 15 Jun 2026 13:56:18 -0700 Subject: [PATCH] Add Windows unified exec yield floor (#27086) ## Why The Windows `unified_exec` experiment regressed at the turn level in a way that points to premature backgrounding / extra command cycles rather than individual responses getting heavier: - `codex_local_tool_calls_per_turn` was up about 20.7%. - `codex_local_blended_tokens_per_turn` was up about 4.1%, and `codex_local_output_tokens_per_turn` was up about 4.0%. - `codex_local_response_latency_per_turn` was up about 8.3%. - The primary activity metrics also moved down: `codex_turns` about -6.6%, `codex_dau` about -1.0%, and `codex_local_hourly_active_users` about -3.0%. At the same time, the per-response metrics moved in the other direction: blended tokens per response, output tokens per response, and latency per response were all lower in test. That suggests the bad turn-level shape is largely about extra tool/model cycles, not each response being slower or more expensive on its own. Local Windows benchmarking showed the likely mechanism: shell-wrapped commands pay a large PowerShell startup/teardown tax before the actual command has much time to run. In the benchmark, the PowerShell wrapper added roughly 0.7-1.0s versus direct exec: - Windows PowerShell: about 740ms p50 / 800ms p90 overhead versus direct exec. - PowerShell 7 (`pwsh`): about 930ms p50 / 980ms p90 overhead versus direct exec. The model commonly asks for a 1s initial yield. On Windows, that can spend nearly the whole window waiting on PowerShell machinery, so otherwise-short commands are more likely to return as background sessions and require follow-up polling/tool calls. This is intentionally a temporary unlock. It gives Windows closer to the same useful post-shell command window as other platforms while we work on reducing the PowerShell tax directly, for example with persistent PowerShell workers or conservative direct-exec paths for commands that do not need shell semantics. ## What changed - Adds a Windows-only 2s floor to `unified_exec`'s initial `yield_time_ms` clamp. - Keeps larger model-requested waits unchanged, including the existing 10s default. - Keeps the existing 30s max clamp. - Leaves non-Windows behavior unchanged. - Adds platform-gated tests for both the Windows floor and the non-Windows clamp behavior. ## Verification - `just test -p codex-core unified_exec` --- codex-rs/core/src/unified_exec/mod.rs | 6 +++++ .../src/unified_exec/process_manager_tests.rs | 27 +++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/codex-rs/core/src/unified_exec/mod.rs b/codex-rs/core/src/unified_exec/mod.rs index 65b18b064..c14d063e9 100644 --- a/codex-rs/core/src/unified_exec/mod.rs +++ b/codex-rs/core/src/unified_exec/mod.rs @@ -62,6 +62,7 @@ pub(crate) use process::SpawnLifecycleHandle; pub(crate) use process::UnifiedExecProcess; pub(crate) const MIN_YIELD_TIME_MS: u64 = 250; +pub(crate) const WINDOWS_INITIAL_EXEC_YIELD_TIME_FLOOR_MS: u64 = 2_000; // Minimum yield time for an empty `write_stdin`. pub(crate) const MIN_EMPTY_YIELD_TIME_MS: u64 = 5_000; pub(crate) const MAX_YIELD_TIME_MS: u64 = 30_000; @@ -165,6 +166,11 @@ struct ProcessEntry { } pub(crate) fn clamp_yield_time(yield_time_ms: u64) -> u64 { + let yield_time_ms = if cfg!(windows) { + yield_time_ms.max(WINDOWS_INITIAL_EXEC_YIELD_TIME_FLOOR_MS) + } else { + yield_time_ms + }; yield_time_ms.clamp(MIN_YIELD_TIME_MS, MAX_YIELD_TIME_MS) } diff --git a/codex-rs/core/src/unified_exec/process_manager_tests.rs b/codex-rs/core/src/unified_exec/process_manager_tests.rs index d9b3a771b..a84f6345e 100644 --- a/codex-rs/core/src/unified_exec/process_manager_tests.rs +++ b/codex-rs/core/src/unified_exec/process_manager_tests.rs @@ -1,4 +1,5 @@ use super::*; +use crate::unified_exec::clamp_yield_time; use pretty_assertions::assert_eq; use tokio::time::Duration; use tokio::time::Instant; @@ -131,6 +132,32 @@ fn exec_server_process_id_matches_unified_exec_process_id() { assert_eq!(exec_server_process_id(/*process_id*/ 4321), "4321"); } +#[cfg(windows)] +#[test] +fn initial_exec_yield_time_uses_windows_floor() { + let above_max_yield_time_ms = crate::unified_exec::MAX_YIELD_TIME_MS + 1; + + assert_eq!( + clamp_yield_time(/*yield_time_ms*/ 1_000), + crate::unified_exec::WINDOWS_INITIAL_EXEC_YIELD_TIME_FLOOR_MS + ); + assert_eq!(clamp_yield_time(/*yield_time_ms*/ 10_000), 10_000); + assert_eq!( + clamp_yield_time(/*yield_time_ms*/ above_max_yield_time_ms), + crate::unified_exec::MAX_YIELD_TIME_MS + ); +} + +#[cfg(not(windows))] +#[test] +fn initial_exec_yield_time_has_no_platform_floor() { + assert_eq!(clamp_yield_time(/*yield_time_ms*/ 1_000), 1_000); + assert_eq!( + clamp_yield_time(/*yield_time_ms*/ 1), + crate::unified_exec::MIN_YIELD_TIME_MS + ); +} + #[tokio::test] async fn network_denial_fallback_message_names_sandbox_network_proxy() { let message = network_denial_message_for_session(/*session*/ None, /*deferred*/ None).await;