-
Notifications
You must be signed in to change notification settings - Fork 366
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 support for fully asynchronous QueryHandle
s
#7964
Conversation
@rerun-bot full-check |
Started a full build: https://github.com/rerun-io/rerun/actions/runs/11617886895 |
@@ -816,7 +858,9 @@ impl<E: StorageEngineLike> QueryHandle<E> { | |||
Retrofilled(UnitChunkShared), | |||
} | |||
|
|||
let state = self.init(); | |||
// That's a synchronous lock, so make sure we barely lock it. | |||
let state = self.init_(store, cache); |
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.
This looks suspicious to me -- the body of that function is large and I wouldn't expect us to execute that code unless we aren't able to access self.state
.
Why isn't this:
self.state.get_or_init(|| self.init_(store, cache))
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.
Uh-oh, nice catch. I meant to add a non-blocking optimistic check that comes first.
But now that I think about it, this is in QueryHandle
, so realistically nothing will ever contend on this... we can probably just do it all in the OnceLock yeah.
This adds non-blocking methods to all our new shiny storage handles, and uses these new non-blocking primitives to implement asynchronous helpers (read:
Future
s) in ourQueryHandle
.These helpers are then used on the other side to implement a proper
Stream
.In particular, all read locks are now recursive, always.
Recursive locks are mandatory for two reasons:
Checklist
main
build: rerun.io/viewernightly
build: rerun.io/viewerCHANGELOG.md
and the migration guideTo run all checks from
main
, comment on the PR with@rerun-bot full-check
.