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

[C10D] Rewrite TCPStore client send path to minimize amount of syscalls. #100742

Closed
wants to merge 4 commits into from

Conversation

kumpera
Copy link
Contributor

@kumpera kumpera commented May 5, 2023

Stack from ghstack (oldest at bottom):

Accumulate data in a local buffer prior to sending it. This reduces
the number of syscalls and network packets.

We flush every 1440 bytes to cap the amount of temporaty memory.

Accumulate data in a local buffer prior to sending it. This reduces
the number of syscalls and network packets.

We flush every 1440 bytes to cap the amount of temporaty memory.

[ghstack-poisoned]
@pytorch-bot
Copy link

pytorch-bot bot commented May 5, 2023

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/100742

Note: Links to docs will display an error until the docs builds have been completed.

✅ No Failures

As of commit c75bef3:
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

kumpera pushed a commit that referenced this pull request May 5, 2023
Accumulate data in a local buffer prior to sending it. This reduces
the number of syscalls and network packets.

We flush every 1440 bytes to cap the amount of temporaty memory.

ghstack-source-id: bfa1ed9dc274823176647a9c31454ff0f336fa9b
Pull Request resolved: #100742
…t of syscalls."

Accumulate data in a local buffer prior to sending it. This reduces
the number of syscalls and network packets.

We flush every 1440 bytes to cap the amount of temporaty memory.

[ghstack-poisoned]
Copy link
Contributor

@fduwjj fduwjj left a comment

Choose a reason for hiding this comment

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

This is amazing! Thanks!

public:
SendBuffer(detail::TCPClient& client, detail::QueryType cmd)
: client(client) {
buffer.reserve(32); // enough for most commands
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we make this a const or adjustable rather than hard code it?

Copy link
Contributor

@fduwjj fduwjj left a comment

Choose a reason for hiding this comment

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

This is amazing! Thanks!

Copy link
Contributor

@fduwjj fduwjj left a comment

Choose a reason for hiding this comment

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

This is amazing! Thanks!

Rodrigo Kumpera added 2 commits May 9, 2023 14:29
…t of syscalls."

Accumulate data in a local buffer prior to sending it. This reduces
the number of syscalls and network packets.

We flush every 1440 bytes to cap the amount of temporaty memory.

[ghstack-poisoned]
…t of syscalls."

Accumulate data in a local buffer prior to sending it. This reduces
the number of syscalls and network packets.

We flush every 1440 bytes to cap the amount of temporaty memory.

[ghstack-poisoned]
kumpera pushed a commit that referenced this pull request May 31, 2023
Accumulate data in a local buffer prior to sending it. This reduces
the number of syscalls and network packets.

We flush every 1440 bytes to cap the amount of temporaty memory.

ghstack-source-id: ecf08e21a7326f398411f0882854b089094c1cfa
Pull Request resolved: #100742
@kumpera kumpera added topic: performance topic category topic: not user facing topic category module: c10d Issues/PRs related to collective communications and process groups labels Jun 1, 2023
@kumpera
Copy link
Contributor Author

kumpera commented Jun 1, 2023

@pytorchmergebot merge

@pytorch-bot pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Jun 1, 2023
@pytorchmergebot
Copy link
Collaborator

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging
Check the merge workflow status
here

alimoezzi pushed a commit to alimoezzi/pytorch that referenced this pull request Jun 3, 2023
…ls. (pytorch#100742)

Accumulate data in a local buffer prior to sending it. This reduces
the number of syscalls and network packets.

We flush every 1440 bytes to cap the amount of temporaty memory.
Pull Request resolved: pytorch#100742
Approved by: https://github.com/fduwjj
@facebook-github-bot facebook-github-bot deleted the gh/kumpera/31/head branch June 8, 2023 17:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ciflow/trunk Trigger trunk jobs on your pull request Merged module: c10d Issues/PRs related to collective communications and process groups release notes: distributed (c10d) release notes category topic: not user facing topic category topic: performance topic category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants