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

refactor(kademlia): speeding up topology build #2028

Merged
merged 1 commit into from
Jun 10, 2021
Merged

Conversation

mrekucci
Copy link
Contributor

@mrekucci mrekucci commented Jun 8, 2021

Speeds up the topology build by dividing the connect neighbor
method into two separate phases executed in sequential order.
The first phase makes a number of concurrent connections not
bigger than what is considered for the bin to be saturated. The
second phase takes care of the rest of the connection in a
regular manner.


This change is Reviewable

@Eknir
Copy link
Contributor

Eknir commented Jun 9, 2021

I got some data on the speed of Kademlia build up in the branch versus master

Master (11m total):
922: 0
925: depth 1
927: depth 3
931: depth 5
934: depth 15

This branch (3m total):
912: 0
913: depth 3
915: depth 7
916: depth 15

I collected these metrics manually (refreshing page in browser). Would be very nice if we can have metrics for this, so we can also do more proper benchmarking.

Copy link
Contributor

@Eknir Eknir left a comment

Choose a reason for hiding this comment

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

approving based on the fact that this speeds up Kademlia build up with about 3x

@mrekucci mrekucci force-pushed the kad-build-speedup branch 3 times, most recently from a782eb9 to 990ac50 Compare June 9, 2021 13:04
Copy link
Member

@acud acud left a comment

Choose a reason for hiding this comment

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

Reviewed 3 of 3 files at r1.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @metacertain)

@mrekucci mrekucci marked this pull request as ready for review June 10, 2021 07:36
@mrekucci mrekucci merged commit 388256b into master Jun 10, 2021
@mrekucci mrekucci deleted the kad-build-speedup branch June 10, 2021 07:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants