Skip to content

Commit 8bf090f

Browse files
lokokungDawn LUCI CQ
authored andcommitted
[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 <[email protected]> Commit-Queue: Loko Kung <[email protected]> Kokoro: Kokoro <[email protected]>
1 parent 0c20d02 commit 8bf090f

File tree

2 files changed

+14
-8
lines changed

2 files changed

+14
-8
lines changed

src/dawn/wire/client/EventManager.cpp

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
// See the License for the specific language governing permissions and
1313
// limitations under the License.
1414

15-
#include <unordered_map>
15+
#include <map>
1616
#include <utility>
1717
#include <vector>
1818

@@ -47,13 +47,14 @@ std::pair<FutureID, bool> EventManager::TrackEvent(WGPUCallbackMode mode,
4747
void EventManager::ShutDown() {
4848
// Call any outstanding callbacks before destruction.
4949
while (true) {
50-
std::unordered_map<FutureID, TrackedEvent> movedEvents;
50+
std::map<FutureID, TrackedEvent> movedEvents;
5151
mTrackedEvents.Use([&](auto trackedEvents) { movedEvents = std::move(*trackedEvents); });
5252

5353
if (movedEvents.empty()) {
5454
break;
5555
}
5656

57+
// Ordering guaranteed because we are using a sorted map.
5758
for (auto& [futureID, trackedEvent] : movedEvents) {
5859
// Event should be already marked Ready since events are actually driven by
5960
// RequestTrackers (at the time of this writing), which all shut down before this.
@@ -74,6 +75,7 @@ void EventManager::SetFutureReady(FutureID futureID) {
7475
}
7576

7677
void EventManager::ProcessPollEvents() {
78+
// Since events are already stored in an ordered map, this list must already be ordered.
7779
std::vector<TrackedEvent> eventsToCompleteNow;
7880

7981
// TODO(crbug.com/dawn/2060): EventManager shouldn't bother to track ProcessEvents-type events
@@ -120,7 +122,9 @@ WGPUWaitStatus EventManager::WaitAny(size_t count, WGPUFutureWaitInfo* infos, ui
120122
return WGPUWaitStatus_Success;
121123
}
122124

123-
std::vector<TrackedEvent> eventsToCompleteNow;
125+
// Since the user can specify the FutureIDs in any order, we need to use another ordered map
126+
// here to ensure that the result is ordered for JS event ordering.
127+
std::map<FutureID, TrackedEvent> eventsToCompleteNow;
124128
bool anyCompleted = false;
125129
const FutureID firstInvalidFutureID = mNextFutureID;
126130
mTrackedEvents.Use([&](auto trackedEvents) {
@@ -141,15 +145,15 @@ WGPUWaitStatus EventManager::WaitAny(size_t count, WGPUFutureWaitInfo* infos, ui
141145
if (event.mReady) {
142146
anyCompleted = true;
143147
if (event.mCallback) {
144-
eventsToCompleteNow.emplace_back(std::move(event));
148+
eventsToCompleteNow.emplace(it->first, std::move(event));
145149
}
146150
trackedEvents->erase(it);
147151
}
148152
}
149153
});
150154

151155
// TODO(crbug.com/dawn/2066): Guarantee the event ordering from the JS spec.
152-
for (TrackedEvent& event : eventsToCompleteNow) {
156+
for (auto& [_, event] : eventsToCompleteNow) {
153157
DAWN_ASSERT(event.mReady && event.mCallback);
154158
// .completed has already been set to true (before the callback, per API contract).
155159
event.mCallback(EventCompletionType::Ready);

src/dawn/wire/client/EventManager.h

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717

1818
#include <cstddef>
1919
#include <functional>
20-
#include <unordered_map>
20+
#include <map>
2121
#include <utility>
2222

2323
#include "dawn/common/FutureUtils.h"
@@ -70,8 +70,10 @@ class EventManager final : NonMovable {
7070

7171
Client* mClient;
7272

73-
// Tracks all kinds of events (for both WaitAny and ProcessEvents).
74-
MutexProtected<std::unordered_map<FutureID, TrackedEvent>> mTrackedEvents;
73+
// Tracks all kinds of events (for both WaitAny and ProcessEvents). We use an ordered map so
74+
// that in most cases, event ordering is already implicit when we iterate the map. (Not true for
75+
// WaitAny though because the user could specify the FutureIDs out of order.)
76+
MutexProtected<std::map<FutureID, TrackedEvent>> mTrackedEvents;
7577
std::atomic<FutureID> mNextFutureID = 1;
7678
};
7779

0 commit comments

Comments
 (0)