[WGPUFuture] Updates TODOs with new bugs for easier tracking.
Bug: dawn:1987
Change-Id: I7fcaa1dc27ac4939e2374e836b7d4af787bf00ee
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/151240
Reviewed-by: Austin Eng <enga@chromium.org>
Auto-Submit: Loko Kung <lokokung@google.com>
Commit-Queue: Austin Eng <enga@chromium.org>
Kokoro: Kokoro <noreply+kokoro@google.com>
diff --git a/dawn.json b/dawn.json
index 372202d..7f4d4c4 100644
--- a/dawn.json
+++ b/dawn.json
@@ -29,7 +29,7 @@
"create instance": {
"category": "function",
"returns": "instance",
- "_comment": "TODO(crbug.com/dawn/1987): The return type should be nullable; null is returned in error cases.",
+ "_comment": "TODO(crbug.com/dawn/2048): The return type should be nullable; null is returned in error cases.",
"args": [
{"name": "descriptor", "type": "instance descriptor", "annotation": "const*", "optional": true}
]
@@ -1971,7 +1971,7 @@
]
},
"callback mode": {
- "_comment": "TODO(crbug.com/dawn/1987): Change this to an enum, and always return a future (https://github.com/webgpu-native/webgpu-headers/issues/199#issuecomment-1710784711).",
+ "_comment": "TODO(crbug.com/dawn/2052): Change this to an enum, and always return a future.",
"category": "bitmask",
"values": [
{"name": "future", "value": 1},
@@ -1987,7 +1987,7 @@
},
"wait status": {
"category": "enum",
- "_comment": "TODO(crbug.com/dawn/1987): This could be possibly be [[nodiscard]].",
+ "_comment": "TODO(crbug.com/dawn/2053): This could possibly be [[nodiscard]].",
"emscripten_no_enum_table": true,
"values": [
{"name": "success", "value": 0},
diff --git a/src/dawn/native/EventManager.cpp b/src/dawn/native/EventManager.cpp
index e03deb3..f791477 100644
--- a/src/dawn/native/EventManager.cpp
+++ b/src/dawn/native/EventManager.cpp
@@ -241,7 +241,7 @@
if (future.ready) {
// Set completed before calling the callback.
infos[future.indexInInfos].completed = true;
- // TODO(crbug.com/dawn/1987): Guarantee the event ordering from the JS spec.
+ // TODO(crbug.com/dawn/2066): Guarantee the event ordering from the JS spec.
ASSERT(future.event->mCallbackMode & WGPUCallbackMode_Future);
future.event->EnsureComplete(EventCompletionType::Ready);
}
diff --git a/src/dawn/native/EventManager.h b/src/dawn/native/EventManager.h
index 7eba4eb..b635099 100644
--- a/src/dawn/native/EventManager.h
+++ b/src/dawn/native/EventManager.h
@@ -38,14 +38,17 @@
// entrypoints. All events from this instance (regardless of whether from an adapter, device, queue,
// etc.) are tracked here, and used by the instance-wide ProcessEvents and WaitAny entrypoints.
//
-// TODO(crbug.com/dawn/1987): Can this eventually replace CallbackTaskManager?
-// TODO(crbug.com/dawn/1987): There are various ways to optimize ProcessEvents/WaitAny:
-// - Only pay attention to the earliest serial on each queue.
-// - Spontaneously set events as "early-ready" in other places when we see serials advance, e.g.
+// TODO(crbug.com/dawn/2050): Can this eventually replace CallbackTaskManager?
+//
+// There are various ways to optimize ProcessEvents/WaitAny:
+// - TODO(crbug.com/dawn/2064) Only pay attention to the earliest serial on each queue.
+// - TODO(crbug.com/dawn/2059) Spontaneously set events as "early-ready" in other places when we see
+// serials advance, e.g.
// Submit, or when checking a later wait before an earlier wait.
-// - For thread-driven events (async pipeline compilation and Metal queue events), defer tracking
-// for ProcessEvents until the event is already completed.
-// - Avoid creating OS events until they're actually needed (see the todo in TrackedEvent).
+// - TODO(crbug.com/dawn/2049) For thread-driven events (async pipeline compilation and Metal queue
+// events), defer tracking for ProcessEvents until the event is already completed.
+// - TODO(crbug.com/dawn/2051) Avoid creating OS events until they're actually needed (see the todo
+// in TrackedEvent).
class EventManager final : NonMovable {
public:
EventManager();
@@ -120,7 +123,7 @@
// This creates a temporary ref cycle (Device->Instance->EventManager->TrackedEvent).
// This is OK because the instance will clear out the EventManager on shutdown.
- // TODO(crbug.com/dawn/1987): This is a bit fragile. Is it possible to remove the ref cycle?
+ // TODO(crbug.com/dawn/2067): This is a bit fragile. Is it possible to remove the ref cycle?
Ref<DeviceBase> mDevice;
WGPUCallbackModeFlags mCallbackMode;
@@ -131,7 +134,7 @@
private:
friend class EventManager;
- // TODO(crbug.com/dawn/1987): Optimize by creating an SystemEventReceiver only once actually
+ // TODO(crbug.com/dawn/2051): Optimize by creating an SystemEventReceiver only once actually
// needed (the user asks for a timed wait or an OS event handle). This should be generally
// achievable:
// - For thread-driven events (async pipeline compilation and Metal queue events), use a mutex
diff --git a/src/dawn/native/Queue.cpp b/src/dawn/native/Queue.cpp
index 6d86b2b..0d8acd0 100644
--- a/src/dawn/native/Queue.cpp
+++ b/src/dawn/native/Queue.cpp
@@ -201,7 +201,7 @@
~WorkDoneEvent() override { EnsureComplete(EventCompletionType::Shutdown); }
- // TODO(crbug.com/dawn/1987): When adding support for mixed sources, return false here when
+ // TODO(crbug.com/dawn/2062): When adding support for mixed sources, return false here when
// the device has the mixed sources feature enabled, and so can expose the fence as an OS event.
bool MustWaitUsingDevice() const override { return true; }
@@ -287,7 +287,7 @@
}
WGPUFuture QueueBase::APIOnSubmittedWorkDoneF(const WGPUQueueWorkDoneCallbackInfo& callbackInfo) {
- // TODO(crbug.com/dawn/1987): Once we always return a future, change this to log to the instance
+ // TODO(crbug.com/dawn/2052): Once we always return a future, change this to log to the instance
// (note, not raise a validation error to the device) and return the null future.
ASSERT(callbackInfo.nextInChain == nullptr);
@@ -295,7 +295,7 @@
WGPUQueueWorkDoneStatus validationEarlyStatus;
if (GetDevice()->ConsumedError(ValidateOnSubmittedWorkDone(0, &validationEarlyStatus))) {
- // TODO(crbug.com/dawn/1987): This is here to pretend that things succeed when the device is
+ // TODO(crbug.com/dawn/2021): This is here to pretend that things succeed when the device is
// lost. When the old OnSubmittedWorkDone is removed then we can update
// ValidateOnSubmittedWorkDone to just return the correct thing here.
if (validationEarlyStatus == WGPUQueueWorkDoneStatus_DeviceLost) {
@@ -315,7 +315,7 @@
}
SystemEventReceiver QueueBase::InsertWorkDoneEvent() {
- // TODO(crbug.com/dawn/1987): Implement this in all backends and remove this default impl
+ // TODO(crbug.com/dawn/2058): Implement this in all backends and remove this default impl
CHECK(false);
}
diff --git a/src/dawn/native/SystemEvent.cpp b/src/dawn/native/SystemEvent.cpp
index c1ad4a0..670db2a 100644
--- a/src/dawn/native/SystemEvent.cpp
+++ b/src/dawn/native/SystemEvent.cpp
@@ -164,7 +164,7 @@
bool WaitAnySystemEvent(size_t count, TrackedFutureWaitInfo* futures, Nanoseconds timeout) {
#if DAWN_PLATFORM_IS(WINDOWS)
- // TODO(crbug.com/dawn/1987): Implement this.
+ // TODO(crbug.com/dawn/2054): Implement this.
CHECK(false);
#elif DAWN_PLATFORM_IS(POSIX)
std::vector<pollfd> pollfds(count);
diff --git a/src/dawn/native/metal/QueueMTL.h b/src/dawn/native/metal/QueueMTL.h
index 49dcc12..1cde6f9 100644
--- a/src/dawn/native/metal/QueueMTL.h
+++ b/src/dawn/native/metal/QueueMTL.h
@@ -69,7 +69,7 @@
NSPRef<id> mMtlSharedEvent = nullptr;
// This mutex must be held to access mWaitingEvents (which may happen in a Metal driver thread).
- // TODO(crbug.com/dawn/1987): if we atomically knew a conservative lower bound on the
+ // TODO(crbug.com/dawn/2065): If we atomically knew a conservative lower bound on the
// mWaitingEvents serials, we could avoid taking this lock sometimes. Optimize if needed.
// See old draft code: https://dawn-review.googlesource.com/c/dawn/+/137502/29
MutexProtected<SerialQueue<ExecutionSerial, SystemEventPipeSender>> mWaitingEvents;
diff --git a/src/dawn/native/metal/QueueMTL.mm b/src/dawn/native/metal/QueueMTL.mm
index 635fc04..d0be4b5 100644
--- a/src/dawn/native/metal/QueueMTL.mm
+++ b/src/dawn/native/metal/QueueMTL.mm
@@ -75,7 +75,7 @@
SystemEventReceiver Queue::InsertWorkDoneEvent() {
ExecutionSerial serial = GetScheduledWorkDoneSerial();
- // TODO(crbug.com/dawn/1987): Optimize to not create a pipe for every WorkDone/MapAsync event.
+ // TODO(crbug.com/dawn/2051): Optimize to not create a pipe for every WorkDone/MapAsync event.
// Possible ways to do this:
// - Don't create the pipe until needed (see the todo on TrackedEvent::mReceiver).
// - Dedup event pipes when one serial is needed for multiple events (and add a
diff --git a/src/dawn/tests/end2end/EventTests.cpp b/src/dawn/tests/end2end/EventTests.cpp
index 1b19de3..ad96b87 100644
--- a/src/dawn/tests/end2end/EventTests.cpp
+++ b/src/dawn/tests/end2end/EventTests.cpp
@@ -412,7 +412,7 @@
// Callback should have been called immediately because we leaked it since there's no way to
// call WaitAny or ProcessEvents anymore.
//
- // TODO(crbug.com/dawn/1987): Once Spontaneous is implemented, this should no longer expect the
+ // TODO(crbug.com/dawn/2059): Once Spontaneous is implemented, this should no longer expect the
// callback to be cleaned up immediately (and should expect it to happen on a future Tick).
ASSERT_EQ(status, WGPUQueueWorkDoneStatus_Unknown);
}
@@ -438,7 +438,7 @@
// Callback should have been called immediately because we leaked it since there's no way to
// call WaitAny or ProcessEvents anymore.
//
- // TODO(crbug.com/dawn/1987): Once Spontaneous is implemented, this should no longer expect the
+ // TODO(crbug.com/dawn/2059): Once Spontaneous is implemented, this should no longer expect the
// callback to be cleaned up immediately (and should expect it to happen on a future Tick).
ASSERT_EQ(status, WGPUQueueWorkDoneStatus_Unknown);
}
@@ -449,7 +449,7 @@
// - Other tests?
DAWN_INSTANTIATE_TEST_P(EventCompletionTests,
- // TODO(crbug.com/dawn/1987): Enable tests for the rest of the backends.
+ // TODO(crbug.com/dawn/2058): Enable tests for the rest of the backends.
{MetalBackend()},
{
WaitTypeAndCallbackMode::TimedWaitAny_Future,
@@ -459,11 +459,11 @@
WaitTypeAndCallbackMode::SpinProcessEvents_ProcessEvents,
WaitTypeAndCallbackMode::SpinProcessEvents_ProcessEventsSpontaneous,
- // TODO(crbug.com/dawn/1987): The cases with the Spontaneous flag
+ // TODO(crbug.com/dawn/2059): The cases with the Spontaneous flag
// enabled were added before we implemented all of the spontaneous
// completions. They might accidentally be overly strict.
- // TODO(crbug.com/dawn/1987): Make guarantees that Spontaneous callbacks
+ // TODO(crbug.com/dawn/2059): Make guarantees that Spontaneous callbacks
// get called (as long as you're hitting "checkpoints"), and add the
// corresponding tests, for example:
// - SpinProcessEvents_Spontaneous,
@@ -564,7 +564,7 @@
}
DAWN_INSTANTIATE_TEST(WaitAnyTests,
- // TODO(crbug.com/dawn/1987): Enable tests for the rest of the backends.
+ // TODO(crbug.com/dawn/2058): Enable tests for the rest of the backends.
MetalBackend());
} // anonymous namespace
diff --git a/src/dawn/wire/client/Client.h b/src/dawn/wire/client/Client.h
index 7828535..dd1ed59 100644
--- a/src/dawn/wire/client/Client.h
+++ b/src/dawn/wire/client/Client.h
@@ -108,6 +108,7 @@
MemoryTransferService* mMemoryTransferService = nullptr;
std::unique_ptr<MemoryTransferService> mOwnedMemoryTransferService = nullptr;
PerObjectType<LinkedList<ObjectBase>> mObjects;
+ // TODO(crbug.com/dawn/2061) Eventually we want an EventManager per instance not per client.
EventManager mEventManager;
bool mDisconnected = false;
};
diff --git a/src/dawn/wire/client/EventManager.cpp b/src/dawn/wire/client/EventManager.cpp
index efa4902..5ce0835 100644
--- a/src/dawn/wire/client/EventManager.cpp
+++ b/src/dawn/wire/client/EventManager.cpp
@@ -71,13 +71,13 @@
TrackedEvent& trackedEvent = trackedEvents->at(futureID); // Asserts futureID is in the map
trackedEvent.mReady = true;
});
- // TODO(crbug.com/dawn/1987): Handle spontaneous completions.
+ // TODO(crbug.com/dawn/2059): Handle spontaneous completions.
}
void EventManager::ProcessPollEvents() {
std::vector<TrackedEvent> eventsToCompleteNow;
- // TODO(crbug.com/dawn/1987): EventManager shouldn't bother to track ProcessEvents-type events
+ // TODO(crbug.com/dawn/2060): EventManager shouldn't bother to track ProcessEvents-type events
// until they've completed. We can queue them up when they're received on the wire. (Before that
// point, the RequestTracker tracks them. If/when we merge this with RequestTracker, then we'll
// track both here but still don't need to queue them for ProcessEvents until they complete.)
@@ -148,7 +148,7 @@
}
});
- // TODO(crbug.com/dawn/1987): Guarantee the event ordering from the JS spec.
+ // TODO(crbug.com/dawn/2066): Guarantee the event ordering from the JS spec.
for (TrackedEvent& event : eventsToCompleteNow) {
ASSERT(event.mReady && event.mCallback);
// .completed has already been set to true (before the callback, per API contract).
diff --git a/src/dawn/wire/client/EventManager.h b/src/dawn/wire/client/EventManager.h
index 0b689d6..e74f20b 100644
--- a/src/dawn/wire/client/EventManager.h
+++ b/src/dawn/wire/client/EventManager.h
@@ -37,7 +37,7 @@
// entrypoints. All events from this instance (regardless of whether from an adapter, device, queue,
// etc.) are tracked here, and used by the instance-wide ProcessEvents and WaitAny entrypoints.
//
-// TODO(crbug.com/dawn/1987): This should probably be merged together with RequestTracker.
+// TODO(crbug.com/dawn/2060): This should probably be merged together with RequestTracker.
class EventManager final : NonMovable {
public:
explicit EventManager(Client*);
diff --git a/src/dawn/wire/client/Instance.cpp b/src/dawn/wire/client/Instance.cpp
index fe82dbd..3c79452 100644
--- a/src/dawn/wire/client/Instance.cpp
+++ b/src/dawn/wire/client/Instance.cpp
@@ -120,7 +120,7 @@
}
void Instance::ProcessEvents() {
- // TODO(crbug.com/dawn/1987): This should only process events for this Instance, not others
+ // TODO(crbug.com/dawn/2061): This should only process events for this Instance, not others
// on the same client. When EventManager is moved to Instance, this can be fixed.
GetClient()->GetEventManager()->ProcessPollEvents();
diff --git a/src/dawn/wire/client/Queue.cpp b/src/dawn/wire/client/Queue.cpp
index 70f505c..2f34153 100644
--- a/src/dawn/wire/client/Queue.cpp
+++ b/src/dawn/wire/client/Queue.cpp
@@ -53,7 +53,7 @@
}
WGPUFuture Queue::OnSubmittedWorkDoneF(const WGPUQueueWorkDoneCallbackInfo& callbackInfo) {
- // TODO(crbug.com/dawn/1987): Once we always return a future, change this to log to the instance
+ // TODO(crbug.com/dawn/2052): Once we always return a future, change this to log to the instance
// (note, not raise a validation error to the device) and return the null future.
ASSERT(callbackInfo.nextInChain == nullptr);
