Skip to content
This repository has been archived by the owner on Sep 15, 2021. It is now read-only.

feat: suppport tokio backend #21

Merged
merged 104 commits into from
Sep 9, 2021
Merged

feat: suppport tokio backend #21

merged 104 commits into from
Sep 9, 2021

Conversation

shjwudp
Copy link
Member

@shjwudp shjwudp commented Sep 7, 2021

This commit contains two features and one bug fix.

Features:

  1. Support tokio backend for better performance in send / recv hybrid communication scenarios.
  2. Improved packet split method utils::chunk_size. This method can divide the communication packet into multiple small packets of equal size as required, and the packet body is not less than the minimum limit.

Bug fix:

  1. In our implementation, one p2p communication corresponds to multiple TCP links. When establishing the link, there was a false assumption that the order in which the server accepts is the same as the order in which the client initiates the connect. Because accept function is take a connection from the pool of tcp links, Instead of one-to-one correspondence with connection. Any way, I fixed this bug.

@shjwudp shjwudp requested review from NOBLES5E and a team September 7, 2021 06:32
@NOBLES5E NOBLES5E changed the title feat: suppport optional tokio backend feat: suppport tokio backend Sep 7, 2021
Copy link
Contributor

@NOBLES5E NOBLES5E left a comment

Choose a reason for hiding this comment

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

Some comments on the PR description (which will be in the commit message when merged):

  1. support tokio backend, in addition to XXXX @shjwudp fill this
  2. fix the multi-stream non-correspondence problem caused by random accept during the initialization process ( cannot understand, please describe the problem more explicitly, it's ok if the description is more than a paragraph but it should be easy to understand ( In the future, such "fix" should be in separate PR
  3. Improved unpacking method (same as 2

Since we noticed that AllReduce and All2All (with large/small messages) have different performance characteristics, we now need to update benchmark results to include these different cases instead of just AllReduce

@NOBLES5E NOBLES5E merged commit 5a74f27 into master Sep 9, 2021
@todo todo bot mentioned this pull request Sep 9, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants