api: replaced Future.done with a sync.Cond #508
+65
−28
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
laines-it
force-pushed
the
laines-it/gh-496-replace-Future-chan-cond-Sync
branch
from
dd9e78b to
f5ce75f
Compare
oleg-jukovec
requested changes
Dec 2, 2025
Collaborator
oleg-jukovec
left a comment
oleg-jukovec
left a comment
There was a problem hiding this comment.
Thanks for the patch. Overall, everything's fine.
I've added some implementation notes, but you need to do a proper rebase first.
Collaborator
|
Please, rebase on the master branch. |
laines-it
force-pushed
the
laines-it/gh-496-replace-Future-chan-cond-Sync
branch
from
f5ce75f to
d238853
Compare
oleg-jukovec
requested changes
Dec 7, 2025
Collaborator
oleg-jukovec
left a comment
oleg-jukovec
left a comment
There was a problem hiding this comment.
Thank you for the patch! I have just a one mirror comment:
This commit reduces allocations. Future.done allocation replaced with - Future.cond (sync.Cond) - Future.finished (atomic.Bool) Other code use Future.isDone() instead (Future.done == nil) check. Added Future.finish() marks Future as done. Future.WaitChan() now creates channel on demand. Closes #496
laines-it
force-pushed
the
laines-it/gh-496-replace-Future-chan-cond-Sync
branch
from
d238853 to
948fcee
Compare
oleg-jukovec
approved these changes
Dec 7, 2025
Collaborator
oleg-jukovec
left a comment
oleg-jukovec
left a comment
There was a problem hiding this comment.
Thank you for the patch!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This commit reduces allocations.
Future.done allocation replaced with
Other code use
Future.isDone()insteadFuture.done == nilcheck.Added Future.finish() marks Future as done.
Future.WaitChan() now creates channel on demand.
Closes #496