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

Use sendmmsg to reduce system call overhead #491

Merged
merged 1 commit into from
Nov 21, 2019
Merged

Use sendmmsg to reduce system call overhead #491

merged 1 commit into from
Nov 21, 2019

Conversation

Ralith
Copy link
Collaborator

@Ralith Ralith commented Nov 16, 2019

I noticed sendmsg was 35% of the flame graph for the large streams benchmark, so I finally had a go at this. It yields a roughly 15-30% improvement in each one of the throughput benchmarks! Oddly, sendmmsg is still about 35% of the flame graph, and cranking the batch size way up doesn't seem to help any. We'll probably need to dive into libc or the kernel or even investigate generic segmentation offload or similar to make further headway on this particular bottleneck.

@Ralith
Copy link
Collaborator Author

Ralith commented Nov 16, 2019

Looks like macOS doesn't expose this; there's sendmsg_x, but the comments describe it as unstable. How do we feel about using that?

@Ralith Ralith force-pushed the sendmmsg branch 2 times, most recently from d78d435 to 3734956 Compare November 16, 2019 21:49
@Ralith
Copy link
Collaborator Author

Ralith commented Nov 16, 2019

Using the fallback for now, since sendmsg_x isn't even bound in the libc crate.

@Ralith Ralith force-pushed the sendmmsg branch 2 times, most recently from de70d6d to 8b9df23 Compare November 16, 2019 22:21
@Ralith
Copy link
Collaborator Author

Ralith commented Nov 17, 2019

Investigation suggests that the remaining sendmmsg presence is just how much time it takes to send UDP packets on Linux. Further optimization will have to come from elsewhere, or resort to userspace networking.

Copy link
Member

@djc djc left a comment

Choose a reason for hiding this comment

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

Mostly looking good, some questions.

quinn/src/endpoint.rs Show resolved Hide resolved
quinn/src/platform/fallback.rs Show resolved Hide resolved
quinn/src/platform/unix.rs Outdated Show resolved Hide resolved
@Ralith Ralith force-pushed the sendmmsg branch 2 times, most recently from 623c73d to ae3b0ea Compare November 18, 2019 17:50
@Ralith
Copy link
Collaborator Author

Ralith commented Nov 20, 2019

sendmmsg isn't nearly so prominent in profiles the benchmark introduced in #507 which should be pretty similar to the existing "large streams" benchmark, so maybe criterion is confusing things somehow. The performance benefit seems to stand, however, perhaps in part due to fewer calls into the reactor machinery.

@djc djc merged commit 9b0f175 into master Nov 21, 2019
@delete-merged-branch delete-merged-branch bot deleted the sendmmsg branch November 21, 2019 10:26
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.

2 participants