[WGPUFuture] Use map over unordered_map for JS event ordering.
Bug: dawn:2066
Change-Id: Ie8e720c5e48f3e471a8eb5fe1019bcd0441583ad
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/153000
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Loko Kung <lokokung@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
diff --git a/src/dawn/wire/client/EventManager.cpp b/src/dawn/wire/client/EventManager.cpp
index f567078..1e0f1e0 100644
--- a/src/dawn/wire/client/EventManager.cpp
+++ b/src/dawn/wire/client/EventManager.cpp
@@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.
-#include <unordered_map>
+#include <map>
#include <utility>
#include <vector>
@@ -47,13 +47,14 @@
void EventManager::ShutDown() {
// Call any outstanding callbacks before destruction.
while (true) {
- std::unordered_map<FutureID, TrackedEvent> movedEvents;
+ std::map<FutureID, TrackedEvent> movedEvents;
mTrackedEvents.Use([&](auto trackedEvents) { movedEvents = std::move(*trackedEvents); });
if (movedEvents.empty()) {
break;
}
+ // Ordering guaranteed because we are using a sorted map.
for (auto& [futureID, trackedEvent] : movedEvents) {
// Event should be already marked Ready since events are actually driven by
// RequestTrackers (at the time of this writing), which all shut down before this.
@@ -74,6 +75,7 @@
}
void EventManager::ProcessPollEvents() {
+ // Since events are already stored in an ordered map, this list must already be ordered.
std::vector<TrackedEvent> eventsToCompleteNow;
// TODO(crbug.com/dawn/2060): EventManager shouldn't bother to track ProcessEvents-type events
@@ -120,7 +122,9 @@
return WGPUWaitStatus_Success;
}
- std::vector<TrackedEvent> eventsToCompleteNow;
+ // Since the user can specify the FutureIDs in any order, we need to use another ordered map
+ // here to ensure that the result is ordered for JS event ordering.
+ std::map<FutureID, TrackedEvent> eventsToCompleteNow;
bool anyCompleted = false;
const FutureID firstInvalidFutureID = mNextFutureID;
mTrackedEvents.Use([&](auto trackedEvents) {
@@ -141,7 +145,7 @@
if (event.mReady) {
anyCompleted = true;
if (event.mCallback) {
- eventsToCompleteNow.emplace_back(std::move(event));
+ eventsToCompleteNow.emplace(it->first, std::move(event));
}
trackedEvents->erase(it);
}
@@ -149,7 +153,7 @@
});
// TODO(crbug.com/dawn/2066): Guarantee the event ordering from the JS spec.
- for (TrackedEvent& event : eventsToCompleteNow) {
+ for (auto& [_, event] : eventsToCompleteNow) {
DAWN_ASSERT(event.mReady && event.mCallback);
// .completed has already been set to true (before the callback, per API contract).
event.mCallback(EventCompletionType::Ready);
diff --git a/src/dawn/wire/client/EventManager.h b/src/dawn/wire/client/EventManager.h
index 2260b73..2c599e2 100644
--- a/src/dawn/wire/client/EventManager.h
+++ b/src/dawn/wire/client/EventManager.h
@@ -17,7 +17,7 @@
#include <cstddef>
#include <functional>
-#include <unordered_map>
+#include <map>
#include <utility>
#include "dawn/common/FutureUtils.h"
@@ -70,8 +70,10 @@
Client* mClient;
- // Tracks all kinds of events (for both WaitAny and ProcessEvents).
- MutexProtected<std::unordered_map<FutureID, TrackedEvent>> mTrackedEvents;
+ // Tracks all kinds of events (for both WaitAny and ProcessEvents). We use an ordered map so
+ // that in most cases, event ordering is already implicit when we iterate the map. (Not true for
+ // WaitAny though because the user could specify the FutureIDs out of order.)
+ MutexProtected<std::map<FutureID, TrackedEvent>> mTrackedEvents;
std::atomic<FutureID> mNextFutureID = 1;
};
