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

Update tokio-tungstenite 0.20 #2116

Merged
merged 3 commits into from
Aug 2, 2023
Merged

Conversation

alexheretic
Copy link
Contributor

@alexheretic alexheretic commented Jul 22, 2023

Motivation

More optimal websocket SinkExt::feed performance by using recent upstream versions with better flushing behaviour.

Solution

  • Update tokio-tungstenite to 0.20. See changelog, tungstenite changelog.
  • breaking: Remove WebSocketUpgrade::max_send_queue.
  • Add WebSocketUpgrade::write_buffer_size, max_write_buffer_size (new upstream).
    Note: I added all upstream docs, feel free to trim them down if you prefer.

/// Does nothing, instead use `max_write_buffer_size`.
#[deprecated]
pub fn max_send_queue(self, _: usize) -> Self {
self
Copy link
Member

Choose a reason for hiding this comment

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

You say it’s deprecated upstream but that should mean we can still call it right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can. However, upstream it does absolutely nothing since the send-queue concept was removed.
The field was kept & deprecated to smoothen the churn. It should be removed with the next tungstenite breaking change.

So with no reason to set it, we can just deprecate and have the method do nothing (which also even more clearly demonstrates that it does nothing to axum users). Or as a breaking change remove it.

Copy link
Member

Choose a reason for hiding this comment

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

My 2c: Merge deprecation into v0.6.x (and do another point release when releasing 0.7, if not sooner), remove on main.

Copy link
Member

Choose a reason for hiding this comment

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

That sounds good to me!

@alexheretic do you wanna change this PR to include the breaking changes and make another one into the v0.6.x branch?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. I raised #2123 with the non-breaking update to v0.6.x.

Deprecate WebSocketUpgrade::max_send_queue
Add WebSocketUpgrade::write_buffer_size, max_write_buffer_size
Copy link
Member

@davidpdrsn davidpdrsn left a comment

Choose a reason for hiding this comment

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

Thanks!

@davidpdrsn davidpdrsn merged commit a6a849b into tokio-rs:main Aug 2, 2023
17 checks passed
@alexheretic alexheretic deleted the tungstenite-0.20 branch August 2, 2023 20:19
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