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

UID2-4391 Major refactor to improve performance and maintainability #7

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

atarassov-ttd
Copy link
Contributor

@atarassov-ttd atarassov-ttd commented Nov 6, 2024

- Removed redundant separation of IO and Worker threads keeping a single pool IO worker threads
- Removed memory arena and custom memory management for IO buffers
- Removed half-baked support for multiple inflight output buffers
- Increased IO buffer size to 4096 bytes
- Simplified and cleaned up a lot of code
- Removed support for socks proxy in config as this was not an implemented feature
- Simplified socket implementation, especially around how sockets get closed in case of errors
- Make proxy a bit fairer by round-robining across all connections with a single IO operation at a time, instead of draining each socket in order
- Make support for handling async connections more explicit
- Add unit test coverage, most importantly - for the sockets IO logic
- Improved/corrected README

Manually verified through various tests as well as using tcptunnelchecker.

- Removed redundant separation of IO and Worker threads keeping a single pool IO worker threads
- Removed memory arena and custom memory management for IO buffers
- Removed half-baked support for multiple inflight output buffers
- Increased IO buffer size to 1024 bytes
- Simplified and cleaned up a lot of code
- Removed support for socks proxy in config as this was not an implemented feature
- Simplified socket implementation, especially around how sockets get closed in case of errors
- Make proxy a bit fairer by round-robining across all connections with a single IO operation at a time, instead of draining each socket in order
- Make support for handling async connections more explicit
- Add unit test coverage, most importantly - for the sockets IO logic
- Improved/corrected README

Manually verified through various tests as well as using tcptunnelchecker.
vsock-bridge/src/config.cpp Outdated Show resolved Hide resolved
vsock-bridge/src/socket.cpp Show resolved Hide resolved
vsock-bridge/src/socket.cpp Outdated Show resolved Hide resolved
vsock-bridge/src/channel.cpp Show resolved Hide resolved
@scong-ttd
Copy link
Contributor

scong-ttd commented Nov 6, 2024

  • Increased IO buffer size to 1024 bytes

Shall we increase to more based on uid operator payload size? (or make it configurable)

@atarassov-ttd
Copy link
Contributor Author

  • Increased IO buffer size to 1024 bytes

Shall we increase to more based on uid operator payload size? (or make it configurable)

That was incorrect statement in the PR description, updated to 4096 which is the final number we settled on based on perf testing.

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