improvement(yamux): make the window size configurable

[lchenut]

Make the window size configurable.

[codecov]

Codecov Report

Merging #987 (a05854f) into unstable (ce0685c) will decrease coverage by 0.10%.
Report is 7 commits behind head on unstable.
The diff coverage is 100.00%.

Additional details and impacted files

@@             Coverage Diff              @@
##           unstable     #987      +/-   ##
============================================
- Coverage     83.23%   83.14%   -0.10%     
============================================
  Files            91       91              
  Lines         15328    15335       +7     
============================================
- Hits          12759    12750       -9     
- Misses         2569     2585      +16     

Files	Coverage Δ
libp2p/builders.nim	93.75% <100.00%> (+0.69%)	⬆️
libp2p/muxers/yamux/yamux.nim	89.94% <100.00%> (+0.24%)	⬆️

... and 13 files with indirect coverage changes

[kaiserd]

LGTM :). Thank you.

In the tests, we could check if the B learned the correct updated winodw size from A, after A established a channel.
This is indirectly done by writing the bytes and checking if the other side blocked.
We could also explicitly add the check. (The check by writing bytes would also pass if the receiveWindow is larger than it should be.)

(There is also the unittest version error.)

[diegomrsantos]

This could be relevant https://discuss.libp2p.io/t/optimizing-yamux-flow-control-sending-window-update-frames-early/843. What's our current approach?

[diegomrsantos]

Also potentially relevant libp2p/test-plans#332

[mxinden]

Also potentially relevant libp2p/test-plans#332

The results you are pointing at come from dynamic window sizing. You can find more details in libp2p/rust-yamux#162. Note go-libp2p implemented this in libp2p/go-yamux#9. More to come from the Rust side :)

[diegomrsantos]

Thanks a lot for more context on that, @mxinden.

[kaiserd]

LGTM :). Thank you.

[diegomrsantos]

Why can maxRecvWindow be smaller than recvWindow?

[lchenut]

Because maxRecvWindow could be, for example, 64k but the peer you're connected to will always assume your recvWindow is 256k (or larger if you enlarge it during the SYN/ACK). In this case, the recvWindow will be larger than maxRecvWindow until it receives 192k. Then it'll not update to something larger than 64k.

[kaiserd]

The spec does not really exclude the case that the desired recvWindow size is below 256.
So, you could set maxRecvWindow to something lower than 256, which would be lower than the initial recvWindow size of 256K.
Once the recvWindow falls below maxRecvWindow, it will only ever grow back to maxRecvWindow.

That being said, we should either

1. explain this fact as a comment in the code
2. or exclude that case and force maxRecvWindow to be at least 256k (== recvWindow).

I'd prefer 2), but I do not have a strong opinion here.

@diegomrsantos @lchenut wdyt?
Once we have consensus here, we can merge this PR :).

[diegomrsantos]

My interpretation is that other peers will always assume your initial recvWindow is 256k. The point seems to be if we find it valuable to allow its max size to be lower than this. This IMHO is non-intuitive and requires a comment in the code, at least. In general, I haven't seen convincing practical evidence about why we should do it, but I don't have a strong opinion as well.

[diegomrsantos]

Please, address the comments.