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

blockservice: remove busyloop in getBlocks by removing batching #232

Merged
merged 1 commit into from
Mar 29, 2023

Conversation

Jorropo
Copy link
Contributor

@Jorropo Jorropo commented Mar 28, 2023

It makes the code quite more complex and there is no evidance it helps for anything.

Capture d’écran du 2023-03-29 01-26-46
This reduce the CPU usage of ipfs get (and other code that use the blockservice getBlocks) by ~7x (from 200% CPU usage to 30%).

@Jorropo Jorropo requested a review from a team as a code owner March 28, 2023 23:30
@codecov
Copy link

codecov bot commented Mar 28, 2023

Codecov Report

Merging #232 (1c17d52) into main (5bbb21b) will increase coverage by 0.06%.
The diff coverage is 70.58%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #232      +/-   ##
==========================================
+ Coverage   48.13%   48.19%   +0.06%     
==========================================
  Files         267      267              
  Lines       32571    32565       -6     
==========================================
+ Hits        15678    15695      +17     
+ Misses      15236    15218      -18     
+ Partials     1657     1652       -5     
Impacted Files Coverage Δ
blockservice/blockservice.go 73.56% <70.58%> (-0.97%) ⬇️

... and 10 files with indirect coverage changes

@Jorropo Jorropo requested a review from guseggert March 28, 2023 23:36
@Jorropo Jorropo force-pushed the blockservice-millions-of-block-notifies branch from 2cd198b to 8efc0a4 Compare March 28, 2023 23:40
@Jorropo Jorropo changed the title fix: remove busyloop in blockservice and add tracing fix: remove busyloop in blockservice Mar 28, 2023
@Jorropo Jorropo force-pushed the blockservice-millions-of-block-notifies branch 2 times, most recently from 1cfd8cb to 9289f0e Compare March 29, 2023 00:18
@Jorropo Jorropo changed the title fix: remove busyloop in blockservice blockservice: remove busyloop in getBlocks by removing batching Mar 29, 2023
It makes the code quite more complex and there is no evidance it helps for anything.
@Jorropo Jorropo force-pushed the blockservice-millions-of-block-notifies branch from 9289f0e to 1c17d52 Compare March 29, 2023 04:06
@Jorropo Jorropo force-pushed the blockservice-millions-of-block-notifies branch 2 times, most recently from 890701e to 1c17d52 Compare March 29, 2023 04:31
blockservice/blockservice.go Show resolved Hide resolved
@Jorropo Jorropo enabled auto-merge (rebase) March 29, 2023 22:07
@Jorropo Jorropo merged commit 6f324be into main Mar 29, 2023
@Jorropo Jorropo deleted the blockservice-millions-of-block-notifies branch March 29, 2023 22:07
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.

3 participants