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

100x speedup of EdgeWalk.take_step() by caching results of GigaChannelCommunity.guess_address() #7450

Merged
merged 1 commit into from Jun 1, 2023

Conversation

kozlovsky
Copy link
Collaborator

@kozlovsky kozlovsky commented May 31, 2023

Fixes #7427.

This PR monkey-patches the Community.guess_address method to add 5-second caching of its results.
It allows to speed up EdgeWalk.take_step() (that sometimes blocks the asyncio loop) by 100 times from 3 seconds to 0.03 seconds.

The next version of IPv8 is expected to include proper caching, so this patch can be removed after upgrading to a version of IPv8 that natively implements caching.

@kozlovsky kozlovsky requested a review from a team as a code owner May 31, 2023 14:13
@kozlovsky kozlovsky requested review from xoriole and removed request for a team May 31, 2023 14:13
@xoriole xoriole requested a review from qstokkink May 31, 2023 14:15
@qstokkink
Copy link
Contributor

Since I was requested for review, I have one question about this implementation:

If the symptoms of #7427 are only caused by the EdgeWalk being very aggressive and the only affected community is GigaChannelCommunity, why not just overwrite the method in the subclass (GigaChannelCommunity.guess_address) with your fix instead of monkey-patching the entire Community base class? That seems cleaner.

@kozlovsky
Copy link
Collaborator Author

@qstokkink I reimplemented it as a method of GigaChannelCommunity; it indeed looks cleaner this way

@kozlovsky kozlovsky changed the title 100x speedup of EdgeWalk.take_step() by caching results of Community.get_address() 100x speedup of EdgeWalk.take_step() by caching results of GigaChannelCommunity.guess_address() May 31, 2023
Copy link
Contributor

@qstokkink qstokkink left a comment

Choose a reason for hiding this comment

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

Nice 🙆‍♂️

@kozlovsky kozlovsky merged commit 9c2a7da into Tribler:release/7.13 Jun 1, 2023
16 checks passed
@kozlovsky kozlovsky deleted the fix/netifaces_cache branch June 1, 2023 06:29
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.

None yet

2 participants