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

Fix yamux handling of writes bigger than the window size #295

Merged
merged 3 commits into from
Aug 15, 2023

Conversation

ianopolous
Copy link
Contributor

Also fix inefficient window size handling.

I tried to test this with ping, but of course ping doesn't handle messages sent in multiple parts. But we have tested this downstream.

Fix inefficient window size handling
@mxinden
Copy link
Member

mxinden commented Jul 11, 2023

In case it is of some use, in rust-yamux we split data frames at 16* 1024 bytes.

By limitting the data frame payload size one gains the following
beneftis:

  1. Reduce delays sending time-sensitive frames, e.g. window updates.

  2. Minimize head-of-line blocking across streams.

  3. Enable better interleaving of send and receive operations, as each is
    carried out atomically instead of concurrently with its respective
    counterpart.

Limiting the frame size to 16KiB does not introduce a large overhead. A
Yamux frame header is 12 bytes large, thus this change introduces an
overhead of ~0.07%.

See libp2p/rust-yamux#111 for details.

Release delayed yamux send buffer once sent. Add Unit test
Copy link
Collaborator

@Nashatyrev Nashatyrev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Nashatyrev Nashatyrev merged commit bf15e47 into libp2p:v1.0.0 Aug 15, 2023
2 checks passed
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