api: replaced Future.done with a sync.Cond #508
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
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
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.
| Future.pushes[], Future.ready (#480, #497). | ||
| * `LogAppendPushFailed` replaced with `LogBoxSessionPushUnsupported` (#480) | ||
| * `LogAppendPushFailed` replaced with `LogBoxSessionPushUnsupported` (#480). | ||
| * Removed deprecated `Connection` methods, related interfaces and tests are updated (#479). |
Collaborator
There was a problem hiding this comment.
It looks like rebase artifacts.
Comment on lines
+19
to
+33
| *NOTE*: due to Future.GetTyped() doesn't decode SelectRequest into structure, substitute Connection.GetTyped() following the example: | ||
| ```Go | ||
| var singleTpl = Tuple{} | ||
| err = conn.GetTyped(space, index, key, &singleTpl) | ||
| ``` | ||
| At now became: | ||
| ```Go | ||
| var tpl []Tuple | ||
| err = conn.Do(NewSelectRequest(space). | ||
| Index(index). | ||
| Limit(1). | ||
| Key(key) | ||
| ).GetTyped(&tpl) | ||
| singleTpl := tpl[0] | ||
| ``` |
Comment on lines
+155
to
+157
| if fut.done == nil { | ||
| fut.done = make(chan struct{}) | ||
| } |
Collaborator
There was a problem hiding this comment.
Just a code-style issue.
Suggested change
| if fut.done == nil { | |
| fut.done = make(chan struct{}) | |
| } | |
| if fut.done == nil { | |
| fut.done = make(chan struct{}) | |
| } | |
| close(fut.done) | ||
| } | ||
|
|
||
| fut.cond.Broadcast() |
Collaborator
There was a problem hiding this comment.
Suggested change
| fut.cond.Broadcast() | |
| fut.cond.Broadcast() | |
| case conn.rlimit <- struct{}{}: | ||
| case <-fut.done: | ||
| default: | ||
| <-fut.WaitChan() |
Collaborator
There was a problem hiding this comment.
It should look like this:
Suggested change
| <-fut.WaitChan() | |
| default: | |
| select{ | |
| case conn.rlimit <- struct{}{}: | |
| case <-fut.WaitChan(): | |
| if fut.err == nil { | |
| panic("fut.done is closed, but err is nil") | |
| } | |
| } |
| mutex sync.Mutex | ||
| resp Response | ||
| err error | ||
| cond *sync.Cond |
Collaborator
There was a problem hiding this comment.
Suggested change
| cond *sync.Cond | |
| cond sync.Cond |
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