Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue #6566 - use the demand API for the websocket MessageSinks #6635

Merged
merged 13 commits into from
Aug 31, 2021

Conversation

lachlan-roberts
Copy link
Contributor

Issue #6566

By using the demand API in the websocket MessageSinks we can demand all the frames of a websocket message without succeeding the callback and releasing control of the frame payload buffers. This allows the MessageSinks to not need to copy the payload of each buffer and just store the buffer/callback pair. Now only one copy must be done which is to combine the frame payloads directly before it is delivered to the application.

…amMessageSinkTest failures

Signed-off-by: Lachlan Roberts <[email protected]>
@sbordet
Copy link
Contributor

sbordet commented Aug 18, 2021

@lachlan-roberts can you fix the test failures?

Signed-off-by: Lachlan Roberts <[email protected]>
@lachlan-roberts lachlan-roberts requested a review from gregw August 19, 2021 10:54
@lachlan-roberts lachlan-roberts force-pushed the jetty-10.0.x-6566-WebSocketMessageSinks branch from 102d1b2 to fd6c72c Compare August 24, 2021 00:56
@lachlan-roberts lachlan-roberts requested a review from gregw August 24, 2021 00:56
gregw
gregw previously approved these changes Aug 24, 2021
Copy link
Contributor

@sbordet sbordet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but there are test failures.

@lachlan-roberts lachlan-roberts requested a review from gregw August 30, 2021 05:08
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.

3 participants