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

feat: switch to tungstenite-rs #123

Closed
wants to merge 4 commits into from

Conversation

ctrlaltf24
Copy link
Collaborator

Was going to get in the way, plus has more breaking changes (see changelog changes).

Fixes: #115

@ctrlaltf24 ctrlaltf24 marked this pull request as draft October 2, 2021 00:02
@ctrlaltf24
Copy link
Collaborator Author

tungstenite-rs is a blocking library (that doesn't support .split). Observed significant performance degradation (25s minimum, peak of 75s) in the following case (client::socket::test::socket_io_integration):

  • Callback mode (where there is a separate thread constantly calling poll)
  • Attempt to send packets on the clone of the socket returned
  • Observe that websocket connection is busy reading
  • When a ping packet is received it is possible that the lock switches to the write
  • Socket finally sends a message

(keep in mind this same performance degredation is likely on the secure websocket connection as it also doesn't support .split)

Suggested fix: Use some async internally. Currently we are trying to expose a callback interface (typically async), yet only using blocking libraries. Delay until 0.4.0, as it introduces a general performance degradation.

@1c3t3a
Copy link
Owner

1c3t3a commented Oct 7, 2021

tungstenite-rs is a blocking library (that doesn't support .split). Observed significant performance degradation (25s minimum, peak of 75s) in the following case (client::socket::test::socket_io_integration):

  • Callback mode (where there is a separate thread constantly calling poll)
  • Attempt to send packets on the clone of the socket returned
  • Observe that websocket connection is busy reading
  • When a ping packet is received it is possible that the lock switches to the write
  • Socket finally sends a message

(keep in mind this same performance degredation is likely on the secure websocket connection as it also doesn't support .split)

Suggested fix: Use some async internally. Currently we are trying to expose a callback interface (typically async), yet only using blocking libraries. Delay until 0.4.0, as it introduces a general performance degradation.

Ahhh shoot. That's bad. I already saw that they don't support split but honestly, but I hoped that it doesn't introduce a performance downgrade of that severity. Introducing async code (via tokio-tungstenite) would make sense, but also brings the downside of the full tokio dependency tree. As you said I would defenetly postpone the issue, but we might stick to websocket for the blocking version...

@1c3t3a
Copy link
Owner

1c3t3a commented Jan 28, 2022

Superseded by #146.

@1c3t3a 1c3t3a closed this Jan 28, 2022
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.

Switch websocket library
2 participants