-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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
implement a worker to consolidate HasBlock provide calls into one #860
Conversation
…alieviate memory pressure
case bs.provideKeys <- nextKey: | ||
if len(toprovide) > 0 { | ||
nextKey = toprovide[0] | ||
toprovide = toprovide[1:] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see three select{}
s in here which are very similar.
If you only switch on the bs.provideKeys <- nextKey
after nextKey != nil
, this can be one for { select {}}
IMHO.
ps: s/toprovide/toProvide/
for the pedantics. 😄
if !ok { | ||
log.Debug("newBlocks channel closed") | ||
log.Debug("provideKeys channel closed") | ||
return | ||
} | ||
ctx, _ := context.WithTimeout(ctx, provideTimeout) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmmm i wonder if this cancel func must also be called-- cc @cryptix ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right. Missed that one... we should add this one to #822.
LGTM |
|
implement a worker to consolidate HasBlock provide calls into one
This PR grabs all the keys that need to be provided and consolidates them into a single buffer of keys. In doing so, it releases the reference to the block being provided, allowing the GC to remove it from memory, and also preventing the HasBlock goroutines from building up blocking on provide sends.