Skip to content

Commit 67e6a2e

Browse files
committed
GPU Metal: give the device events CUDA's semantics
The events signalled and waited for the fixed value 1, and ReleaseEvent reset them to 0, so a wait issued after a release, or for an event not recorded yet, never completed: the GPU watchdog killed each such command buffer after about 10 s, and the queue then dropped all later work. On a 5-event pp time frame this made the TPC reconstruction take 80 s and return nothing. Count up per event instead: RecordMarker signals the next value, a wait asks for the last recorded one and is a no-op if there is none, as cudaStreamWaitEvent is, and ReleaseEvent does nothing, as in CUDA.
1 parent ed8e93b commit 67e6a2e

3 files changed

Lines changed: 22 additions & 7 deletions

File tree

‎GPU/GPUTracking/Base/metal/GPUReconstructionMetal.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ class GPUReconstructionMetal : public GPUReconstructionProcessing::KernelInterfa
5959

6060
void CommitStream(int32_t stream);
6161
void DrainStream(int32_t stream);
62+
uint64_t EventValue(deviceEvent ev, bool next);
6263

6364
private:
6465
int32_t AddKernels();

‎GPU/GPUTracking/Base/metal/GPUReconstructionMetal.mm‎

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -291,10 +291,14 @@ MetalMemRef metalResolve(const GPUReconstructionMetalInternals* internals, const
291291

292292
void GPUReconstructionMetal::ReleaseEvent(deviceEvent ev)
293293
{
294-
// FIXME: is this supposed to reset the event for it to be repurposed
295-
// or to decrease the ref count?
296-
auto mtlEvent = (__bridge id<MTLSharedEvent>)(ev.get<void*>());
297-
[mtlEvent setSignaledValue:0];
294+
// Nothing to do, as in CUDA: the next RecordMarker supersedes the last one.
295+
}
296+
297+
uint64_t GPUReconstructionMetal::EventValue(deviceEvent ev, bool next)
298+
{
299+
std::lock_guard<std::mutex> lock(mInternals->eventMutex);
300+
auto& value = mInternals->eventValues[ev.get<void*>()];
301+
return next ? ++value : value;
298302
}
299303

300304
void GPUReconstructionMetal::RecordMarker(deviceEvent* ev, int32_t stream)
@@ -303,7 +307,7 @@ MetalMemRef metalResolve(const GPUReconstructionMetalInternals* internals, const
303307
// Does not change the retain count, so it's important we manage
304308
// the lifetime of the events outside here.
305309
auto mtlEvent = (__bridge id<MTLSharedEvent>)(ev->get<void*>());
306-
[cb encodeSignalEvent:mtlEvent value:1];
310+
[cb encodeSignalEvent:mtlEvent value:EventValue(*ev, true)];
307311
CommitStream(stream);
308312
}
309313

@@ -353,7 +357,9 @@ MetalMemRef metalResolve(const GPUReconstructionMetalInternals* internals, const
353357
id<MTLCommandBuffer> cb = mInternals->commandBuffers[stream];
354358
for (int32_t ei = 0; ei < nEvents; ei++) {
355359
auto mtlEvent = (__bridge id<MTLSharedEvent>)(evList[ei].get<void*>());
356-
[cb encodeWaitForEvent:mtlEvent value:1];
360+
if (const uint64_t value = EventValue(evList[ei], false)) {
361+
[cb encodeWaitForEvent:mtlEvent value:value];
362+
}
357363
}
358364
CommitStream(stream);
359365
DrainStream(stream);
@@ -363,7 +369,7 @@ MetalMemRef metalResolve(const GPUReconstructionMetalInternals* internals, const
363369
{
364370
for (int32_t i = 0; i < nEvents; i++) {
365371
auto mtlEvent = (__bridge id<MTLSharedEvent>)(evList[i].get<void*>());
366-
if (mtlEvent.signaledValue == 0) {
372+
if (mtlEvent.signaledValue < EventValue(evList[i], false)) {
367373
return false;
368374
}
369375
}

‎GPU/GPUTracking/Base/metal/GPUReconstructionMetalIncludesHost.h‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@ using half = _Float16;
3232
#include <vector>
3333
#include <string>
3434
#include <memory>
35+
#include <mutex>
36+
#include <unordered_map>
3537
#include "GPULogging.h"
3638

3739
#include "GPUReconstructionMetal.h"
@@ -67,6 +69,12 @@ struct GPUReconstructionMetalInternals {
6769
id<MTLBuffer> mem_host;
6870

6971
id<MTLLibrary> library;
72+
73+
// The value each event was last recorded with. An MTLSharedEvent only counts
74+
// up, so recording signals the next value and waiting asks for the last one,
75+
// which makes waiting for a never recorded event a no-op, as in CUDA.
76+
std::unordered_map<void*, uint64_t> eventValues;
77+
std::mutex eventMutex;
7078
};
7179
} // namespace o2::gpu
7280

0 commit comments

Comments
 (0)