TooTallNate / Java-WebSocket

A barebones WebSocket client and server implementation written in 100% Java.
http://tootallnate.github.io/Java-WebSocket
MIT License
10.54k stars 2.58k forks source link

Add ability to disable default receive buffer size in websocket server #1406

Closed Vangreen closed 7 months ago

Vangreen commented 7 months ago

Description

In our project we encountered performance drops in websocket connection over vpn. During debugging we find out that when socket.setReceiveBufferSize(WebSocketImpl.RCVBUF); is not set the performance increases (RRT is smaller and more packets per second are ingested)

image

Motivation and Context

Improve performance

How Has This Been Tested?

Types of changes

Checklist:

PhilipRoman commented 7 months ago

Thanks for the PR! I think I would like a more general solution, so I've opened #1407

Vangreen commented 7 months ago

@PhilipRoman We try also this before, but only when ReceiveBufferSize is not set to any value, we achieved the best results, no matter how high the value we set

PhilipRoman commented 7 months ago

@Vangreen that is really strange, could you please verify with your setup that even doing

      socket.setReceiveBufferSize(socket.getReceiveBufferSize());

here: https://github.com/TooTallNate/Java-WebSocket/blob/master/src/main/java/org/java_websocket/server/WebSocketServer.java#L581 causes the slowdown? (and maybe post the value of socket.getReceiveBufferSize() - note the socket prefix, this is the real Socket receive buffer, not the one you set for websocket). For me default buffer size is 65536 and setting buffer size to that value has no effect on performance (compared to not setting at all).

On a side note, the websocket default buffer size of 16K seems too low and causes slowdowns in my testing compared to the system default 64K.

dushabella commented 7 months ago

@PhilipRoman setting up the 'real' socket buffer this way somehow blocks the system a memory management. Let's start from the beginning. After testing the connection with relatively large RTT, we weirdly had a very bad packet processing performance and the reason was too small rwnd. As a result of this huge performence dropdown, we focused on receive buffer size. But despite increasing WebSocketImpl.RCVBUF, on the established conneciton we didn't get SO_RCVBUF greater than 212992.. So enlarging WebSocketImpl.RCVBUF had no effect on tcp window size. Since the system settings where OK and it didn't seem to decrease the SO_RCVBUF, we disabled this line and we received a performance improvement that was dozens of times better.

Since there is only one value WebSocketImpl.RCVBUF for setting up the real socket and application buffer, I would suggest to separate it for the websocket message size and the socket buffer.

marci4 commented 7 months ago

Fixed with #1407

PhilipRoman commented 7 months ago

@dushabella the behaviour you expect is now turned on by default (no explicit set buffer size). I didn't separate the socket and application buffer sizes since there is no point in having read buffer bigger than socket buffer, but also the socket buffer can easily be saturated on a fast connection, so also not a good idea to have it smaller than socket buffer (just extra syscall overhead).

dushabella commented 6 months ago

@PhilipRoman, you're right there may be benefits of setting the same buffer size, but since the SO_RCVBUF and WebSocket layer buffers are logically separate it could be possible to adjust them independently to fine-tune the behavior of the connection. Depending on the network environment, we may want to adjust the buffer sizes for optimal performance/syscalls/memory use.

Consider the connection with relatively small network traffic but a large RTT. Large SO_RCVBUF would be crucial to ensure a proper window size, but it would fill up slowely. Large application buffer wouldn't reduce the number of syscalls since read() shouldn't read out of empty socket anyway, but this large buffer would take up memory and did not use it, and wasted it as a consequence.