-
Notifications
You must be signed in to change notification settings - Fork 131
Implement futures #523
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
base: trunk
Are you sure you want to change the base?
Implement futures #523
Conversation
3bf7093 to
57f9957
Compare
2ca3f09 to
421ad98
Compare
421ad98 to
e184a90
Compare
|
i won't keep force-pushing now that it's open for review - was trying to keep the set of commits small for maintainers' convenience :) i recommend looking at the 2nd and 3rd commits to isolate away the changes from the refactor |
|
|
||
| pub type Context = wgc::global::Global; | ||
| pub struct Context { | ||
| global: wgc::global::Global, |
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 can reduce the diff with impl Deref, let me know if this is preferred :)
| let now = std::time::Instant::now(); | ||
| if now - start >= std::time::Duration::from_nanos(timeout_ns) { | ||
| return native::WGPUWaitStatus_TimedOut; | ||
| } |
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 figured it's simple to keep this as a hot loop than to park the thread, but i can also replace the whole loop with:
let mut success = false;
{
let registry = context.futures.read();
for future in futures.iter_mut() {
// ... the same
}
}
if success {
return native::WGPUWaitStatus_Success;
}
std::thread::sleep(timeout_ns);
let registry = context.futures.read();
for future in futures.iter_mut() {
// ... the same
}
return if success { native::WGPUWaitStatus_Success } else { native::WGPUWaitStatus_TimedOut };| self.futures | ||
| .resize_with(idx as usize + 1, || FutureElement::Vacant); |
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.
eg
let mut reg = FutureRegistry::default();
for _ in 0..253 {
reg.completed_future();
}
// still 0 allocations, `reg.futures.len() == 0`
reg.completed_future();
// 1 allocation, `reg.futures.len() == 255`
reg.incomplete_future();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.
wait no, reg.futures.len() == 1 in the 2nd scenario since the underlying IdentityManager is able to only increment the epoch 😅
|
making this a draft again - i need to make sure to call poll functions appropriately |
This PR implements
wgpuInstanceWaitAnyand modifies already-implemented functions that rely on this functionality.Apologies for the large diff. It's mostly contained to the first commit, which refactors
Contextto be a struct that containswgc::global::Globalinstead of being an alias to that type.Additionally, this PR somewhat replicates logic that's already implemented in
wgc, but are hidden from the public api (aka declared usingpub(crate)and are not re-exported elsewhere). Unfortunately, this includeswgc::lock, which I didn't duplicate here - instead, I simply rely onparking_lotwithout incorporating the feature-gated implementations fromwgc.