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

[WebSockets] Very slow put packet in server->client direction when using TLS WebSocket connection #98176

Closed
Koyper opened this issue Oct 14, 2024 · 3 comments · Fixed by #98995

Comments

@Koyper
Copy link
Contributor

Koyper commented Oct 14, 2024

Tested versions

Reproducible in 4.3 builds and earlier. Tested using current wsl_peer.cpp master for 4.4.

System information

MacOS 13.6.5

Issue description

When a WebSocketPeer is connected using StreamPeerTLS, the speed of putting a packet in the direction of server->client is MUCH slower than in the client->server direction. Without TLS, the speeds are the same in both directions.

I'm using the latest improvements that should be ensuring TCP_NODELAY is always set, but could this not be propogating when using StreamPeerTLS? I'm also redundantly setting set_no_delay(true) in the MRP and that doesn't change the behavior.

In the MRP when using TLS, transferring a 215kb packet client-to-server takes 5msec, and up to 167msecs server-to-client. Without TLS, the transfer is symetrical at about 2msecs in both directions.

The delay/slowdown is consistant regardless of the max packet size or size of the packet, although the delay increases the larger the packet.

I modified the WebSocket Chat demo project to transfer a binary packet in both directions and report the transfer time. The TLS option can be toggled on an off for the WebSocketServer class using the Inspector.

Here is WITHOUT TLS:

Screen.Recording.2024-10-14.at.10.51.22.AM.mov

And here is WITH TLS:

Screen.Recording.2024-10-14.at.10.52.22.AM.mov

Steps to reproduce

See MRP.

Minimal reproduction project (MRP)

websocket_chat_TLS_transfer_speed_bug.zip

@Koyper
Copy link
Contributor Author

Koyper commented Oct 16, 2024

Unlike StreamPeerTCP, StreamPeerTLS has no set_no_delay() method. Does this mean that after the connection is opened, calls to set_no_delay have no effect? Is this a matter of exposing this setting for mbedTLS?

@Faless
Copy link
Collaborator

Faless commented Oct 25, 2024

Unlike StreamPeerTCP, StreamPeerTLS has no set_no_delay() method. Does this mean that after the connection is opened, calls to set_no_delay have no effect? Is this a matter of exposing this setting for mbedTLS?

set_no_delay is an option of the TCP socket which is why there is no such option for the TLS stream.

The WSLPeer is setting that option automatically for both server and client since Godot 4.3, no matter if it's using TLS or not (in the case of TLS, it's setting it in the underlying TCP connection).

So the issue might be somewhere else.

@Faless
Copy link
Collaborator

Faless commented Nov 9, 2024

@Koyper I've opened #98995 which should fix this issue, let me know if you have a chance to try it out.

@akien-mga akien-mga added this to the 4.4 milestone Nov 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants