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

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

Conversation

Vangreen
Copy link

@Vangreen Vangreen commented Apr 9, 2024

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

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@PhilipRoman
Copy link
Collaborator

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

@Vangreen
Copy link
Author

@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
Copy link
Collaborator

PhilipRoman commented Apr 22, 2024

@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
Copy link

@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
Copy link
Collaborator

marci4 commented Apr 27, 2024

Fixed with #1407

@marci4 marci4 closed this Apr 27, 2024
@PhilipRoman
Copy link
Collaborator

@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
Copy link

dushabella commented May 7, 2024

@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.

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.

4 participants