Fix websocket emit-queue starvation when emit() returns None - #430
Open
dr-regier wants to merge 1 commit into
Open
Fix websocket emit-queue starvation when emit() returns None#430dr-regier wants to merge 1 commit into
dr-regier wants to merge 1 commit into
Conversation
`WebSocketHandler._emit_to_queue` enqueued whatever `emit()` returned with no guard. When a handler is idle, `emit()` returns `None` instantly on every call, so with an unbounded queue and no yield on the idle path the loop spins thousands of times/sec enqueuing `None`. This floods the queue and starves the consumer `_emit_loop`, delaying frames already at the head of the queue. Real-world impact (telephony): up to ~18s of intermittent dead air before an already-generated greeting reached the caller. On the idle path, back off 20ms and skip the enqueue. `_emit_loop` already skips `None` outputs, so enqueuing `None` never produced a frame - this only removes wasted work and yields the loop to the consumer; no downstream behavior changes. Related: gradio-app#203
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
WebSocketHandler._emit_to_queueenqueues whateveremit()returns with no guard:When a handler is idle,
emit()returnsNoneinstantly on every call. With an unbounded queue and no yield on the idle path, this spins thousands of times/sec enqueuingNone, floods the queue (observed ~164k depth), and starves the consumer_emit_loopso it isn't scheduled to send the frame already at the head of the queue.Real-world impact (telephony): up to ~18s of intermittent dead air before an already-generated greeting reached the caller. Related: #203, where the same starvation is worked around by adding
await asyncio.sleep(0.01)inside the user'semit().Fix
On the idle path (
output is None), back off 20ms and skip the enqueue:Why this is safe
_emit_loopalready skipsNoneoutputs, so enqueuingNonenever produced a frame - it only created work and event-loop contention. This removes wasted work and yields the loop to the consumer on idle; no downstream behavior changes.Validation
On the live phone path, first-frame send dropped from ~18.6s to ~0.1-0.4s with queue depth ~0 on every call. Tested on
0.0.33; the affected code is identical onmain.Notes / alternatives
emit()await requirement instead of handling it here. Went with the loop-side guard because it's minimal and fixes the existing AsyncStreamHandler breaks when noawaitis used inemit()#203 workaround at the source. Happy to reshape.