⚡ [performance] Unblock event loop in waitForSandboxReady - #39
Conversation
Replaced the synchronous `sleep(delaySeconds)` with `await new Promise(...)` to avoid blocking the Node.js event loop during sandbox readiness checks.
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Replaced the synchronous `sleep(delaySeconds)` with `await new Promise(...)` to avoid blocking the Node.js event loop during sandbox readiness checks. Signed-off-by: Jules <161369871+google-labs-jules[bot]@users.noreply.github.com>
Replaced the synchronous `sleep(delaySeconds)` with `await new Promise(...)` to avoid blocking the Node.js event loop during sandbox readiness checks. Signed-off-by: Jules <161369871+google-labs-jules[bot]@users.noreply.github.com>
Replaced the synchronous `sleep(delaySeconds)` with `await new Promise(...)` to avoid blocking the Node.js event loop during sandbox readiness checks. Signed-off-by: Jules <161369871+google-labs-jules[bot]@users.noreply.github.com>
1 participant
💡 What:
Replaced the synchronous
sleep(delaySeconds)call inwaitForSandboxReadywith an asynchronousawait new Promise(r => setTimeout(r, delaySeconds * 1000)). The function signature was updated to be async, and all callers were updated toawaitit.🎯 Why:
The previous implementation used a synchronous wait primitive that completely blocked the Node.js main thread (via
Atomics.wait). Because this polling loop runs over network-bound checks (waiting for a Kubernetes pod to transition to "Running"), completely halting the JavaScript event loop is highly detrimental. It prevents other asynchronous I/O, timers, or promise continuations from executing in the background, starving the application.📊 Measured Improvement:
In a benchmark comparing the synchronous wait versus the asynchronous wait:
This ensures concurrent network requests, timeouts, and user interactions in the CLI or library can proceed unabated.
PR created automatically by Jules for task 8442760639923651774 started by @Hardonian