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

feat: pre-emptive stream creation for protocols #1516

Merged
merged 21 commits into from
Sep 4, 2023

Conversation

danisharora099
Copy link
Collaborator

@danisharora099 danisharora099 commented Aug 30, 2023

We currently create streams with remote peers for a protocol lazily. ie, for eg, when lightpush.send() is called, a stream is created then - thus adding to the time delays for the send() function.

This was also flagged as a problem in #1465.

This PR thus introduces the concept of optimistic streams for protocols. The idea is to open streams with peers upon connection for protocols optimistically in advance. These existing streams can then be used for requests, instead of creating new ones adhoc.

Results:

Messages sent at an interval of 200ms rapidly over lightpush to the node /dns4/node-01.ac-cn-hongkong-c.wakuv2.test.statusim.net/tcp/8000/wss/p2p/16Uiu2HAkvWiyFsgRhuJEb9JfjYxEkoHLgnUQmr1N5mKWnYjxYRVm:

Now:

lightpush: 293.958ms
lightpush: 166.278ms
lightpush: 266.626ms
lightpush: 264.433ms
lightpush: 265.46ms
lightpush: 253.402ms

Before:

lightpush: 397.283ms
lightpush: 475.635ms
lightpush: 485.878ms
lightpush: 538.217ms
lightpush: 487.232ms
lightpush: 486.972ms

@github-actions
Copy link

github-actions bot commented Aug 30, 2023

size-limit report 📦

Path Size Loading time (3g) Running time (snapdragon) Total time
Waku core 28.67 KB (0%) 574 ms (0%) 2.2 s (+5.48% 🔺) 2.8 s
Waku Simple Light Node 298.75 KB (+0.1% 🔺) 6 s (+0.1% 🔺) 4.8 s (-6.81% 🔽) 10.8 s
ECIES encryption 28.68 KB (0%) 574 ms (0%) 2.1 s (+15.88% 🔺) 2.6 s
Symmetric encryption 28.69 KB (0%) 574 ms (0%) 1.8 s (+110% 🔺) 2.4 s
DNS discovery 118.6 KB (0%) 2.4 s (0%) 3.1 s (+29.39% 🔺) 5.5 s
Privacy preserving protocols 122.62 KB (0%) 2.5 s (0%) 2.8 s (+0.44% 🔺) 5.2 s
Light protocols 28.68 KB (+0.98% 🔺) 574 ms (+0.98% 🔺) 1.9 s (+28% 🔺) 2.5 s
History retrieval protocols 27.91 KB (+0.98% 🔺) 559 ms (+0.98% 🔺) 1.6 s (-10.03% 🔽) 2.1 s
Deterministic Message Hashing 5.64 KB (0%) 113 ms (0%) 497 ms (+10.99% 🔺) 610 ms

@danisharora099 danisharora099 changed the title feat: single optimistic stream curation feat: optimistic stream creation for protocols Aug 31, 2023
@danisharora099 danisharora099 marked this pull request as ready for review August 31, 2023 13:51
@danisharora099 danisharora099 requested a review from a team as a code owner August 31, 2023 13:51
@danisharora099 danisharora099 requested review from a team and weboko September 1, 2023 08:10
@weboko weboko mentioned this pull request Sep 1, 2023
4 tasks
@fryorcraken
Copy link
Collaborator

"optimistic" is already coined to describe when libp2p client start sending data over a new stream without waiting for the remote peer to confirm stream creation.
Also, this is not "optimistic" but "pre-emptive" so I'd suggest to change the PR title.

@danisharora099 danisharora099 changed the title feat: optimistic stream creation for protocols feat: pre-emptive stream creation for protocols Sep 4, 2023
@danisharora099 danisharora099 merged commit b4f8216 into master Sep 4, 2023
10 of 11 checks passed
@danisharora099 danisharora099 deleted the feat/optimistic-streams-single branch September 4, 2023 04:57
private handlePeerUpdateStreamPool = (evt: CustomEvent<PeerUpdate>): void => {
const peer = evt.detail.peer;
if (peer.protocols.includes(this.multicodec)) {
this.log(`Optimistically opening a stream to ${peer.id.toString()}`);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Pre-emptively

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

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.

4 participants