Skip to content

drpcstream: optimize sendWindow#81

Open
shubhamdhama wants to merge 1 commit into
cockroachdb:mainfrom
shubhamdhama:improve-send-window
Open

drpcstream: optimize sendWindow#81
shubhamdhama wants to merge 1 commit into
cockroachdb:mainfrom
shubhamdhama:improve-send-window

Conversation

@shubhamdhama

Copy link
Copy Markdown

sendWindow has a single acquirer and a single granter, so serializing every credit update behind a mutex adds avoidable contention. Debit credit atomically before waiting and represent a pending acquisition as a negative balance. Grants can then update the balance atomically and wake the acquirer only when the deficit is repaid.

acquire no longer accepts a context, so it does not need channel-based notifications. Replace the channel with a lazily allocated condition variable, keeping the mutex only around waiting and signaling to prevent lost wakeups.

The stream checks termination before acquiring credit, so the fast path does not need a separate closed check. The close error now acts as the terminal marker and is consulted only when an acquisition blocks.

sendWindow has a single acquirer and a single granter, so serializing
every credit update behind a mutex adds avoidable contention. Debit
credit atomically before waiting and represent a pending acquisition as
a negative balance. Grants can then update the balance atomically and
wake the acquirer only when the deficit is repaid.

acquire no longer accepts a context, so it does not need channel-based
notifications. Replace the channel with a lazily allocated condition
variable, keeping the mutex only around waiting and signaling to prevent
lost wakeups.

The stream checks termination before acquiring credit, so the fast path
does not need a separate closed check. The close error now acts as the
terminal marker and is consulted only when an acquisition blocks.

Co-authored-by: Cursor <cursoragent@cursor.com>
@suj-krishnan

suj-krishnan commented Jul 23, 2026

Copy link
Copy Markdown

Thanks for putting this up! Two comments:

  • I was working on simplifying this too yesterday but took a slightly different approach - I thought a channel would be the simpler/idiomatic way to do this since we only ever have a single waiter.
  • I wanted to spend some time thinking about how to handle close yesterday (and was a little unsure, which is why I hadn't published these changes). The change - 'fast path does not consult closed when enough credit is available' - in this PR simplifies things and is okay per se. But I think that checking for close further down in WriteFrame can prevent a frame from being queued up if a close has happened. This fix is not FC specific and would help even in the non-FC path. It is strictly not needed but I feel it is a useful fix.
    My PR - drpcstream: mutex-free sendWindow flow-control primitive #84

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants