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

Better buffering #4280

Closed
Stebalien opened this issue Oct 5, 2017 · 3 comments
Closed

Better buffering #4280

Stebalien opened this issue Oct 5, 2017 · 3 comments
Labels
P0 Critical: Tackled by core team ASAP topic/perf Performance

Comments

@Stebalien
Copy link
Member

We should add buffering to all of our message-based APIs so we avoid sending message fragments. This should alleviate libp2p/go-libp2p#235.

@Stebalien
Copy link
Member Author

I've now tried:

  • A "double buffer" buffering library (write to a buffer while another thread flushes).
  • Manually adding buffering to bitswap to avoid sending messages lengths separate from messages.

No dice. Nothing makes a difference. The thing is, bitswap doesn't send a lot of little messages (to the same peer). We send a lot of little messages to many peers (buffering won't help) and many large messages to a single peer (buffering won't help). Even if we end up sending the message length in a separate packet/secio-message, that extra message is easily amortized by the size of the primary message in most cases.

TL;DR: Dead end.

@Stebalien
Copy link
Member Author

I take this back. Adding double buffering (with a small delay) helps reduce the number of packets quite a bit (testing with wireshark). I believe the issues with my prior testing were:

  1. I was testing bitswap, not stuff like the DHT (tons of small packets). Buffering should help background bandwidth.
  2. Localhost is not the right environment to test on. Reducing the number of packets sent is significantly more important on the open internet.

@Stebalien Stebalien added status/deferred Conscious decision to pause or backlog and removed status/ready Ready to be worked labels Dec 18, 2018
@Stebalien
Copy link
Member Author

Closing in favor of #3155.

@ghost ghost removed the status/deferred Conscious decision to pause or backlog label Dec 18, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P0 Critical: Tackled by core team ASAP topic/perf Performance
Projects
None yet
Development

No branches or pull requests

2 participants