Skip to content

Commit b96b789

Browse files
makortelfwyzard
authored andcommitted
Fix synchronization mistake in CUDAScopedContext (#327)
Check the event creation only after the product constructor has been called. Otherwise, if the stream was idle before, and the constructor queues work to it, the event is not created and downstream will assume that the product is always there (even if it isn't yet).
1 parent fcbb820 commit b96b789

File tree

4 files changed

+19
-15
lines changed

4 files changed

+19
-15
lines changed

CUDADataFormats/Common/interface/CUDAProduct.h

+4-4
Original file line numberDiff line numberDiff line change
@@ -40,14 +40,14 @@ class CUDAProduct: public CUDAProductBase {
4040
friend class CUDAScopedContext;
4141
friend class edm::Wrapper<CUDAProduct<T>>;
4242

43-
explicit CUDAProduct(int device, std::shared_ptr<cuda::stream_t<>> stream, std::shared_ptr<cuda::event_t> event, T data):
44-
CUDAProductBase(device, std::move(stream), std::move(event)),
43+
explicit CUDAProduct(int device, std::shared_ptr<cuda::stream_t<>> stream, T data):
44+
CUDAProductBase(device, std::move(stream)),
4545
data_(std::move(data))
4646
{}
4747

4848
template <typename... Args>
49-
explicit CUDAProduct(int device, std::shared_ptr<cuda::stream_t<>> stream, std::shared_ptr<cuda::event_t> event, Args&&... args):
50-
CUDAProductBase(device, std::move(stream), std::move(event)),
49+
explicit CUDAProduct(int device, std::shared_ptr<cuda::stream_t<>> stream, Args&&... args):
50+
CUDAProductBase(device, std::move(stream)),
5151
data_(std::forward<Args>(args)...)
5252
{}
5353

CUDADataFormats/Common/interface/CUDAProductBase.h

+8-2
Original file line numberDiff line numberDiff line change
@@ -40,12 +40,18 @@ class CUDAProductBase {
4040
cuda::event_t *event() { return event_.get(); }
4141

4242
protected:
43-
explicit CUDAProductBase(int device, std::shared_ptr<cuda::stream_t<>> stream, std::shared_ptr<cuda::event_t> event);
43+
explicit CUDAProductBase(int device, std::shared_ptr<cuda::stream_t<>> stream):
44+
stream_{std::move(stream)},
45+
device_{device}
46+
{}
4447

4548
private:
4649
friend class CUDAScopedContext;
4750

48-
// Intended to be used only from CUDAScopedContext
51+
// The following functions are intended to be used only from CUDAScopedContext
52+
void setEvent(std::shared_ptr<cuda::event_t> event) {
53+
event_ = std::move(event);
54+
}
4955
const std::shared_ptr<cuda::stream_t<>>& streamPtr() const { return stream_; }
5056

5157
bool mayReuseStream() const {

CUDADataFormats/Common/src/CUDAProductBase.cc

-6
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,6 @@
33
#include "FWCore/ServiceRegistry/interface/Service.h"
44
#include "HeterogeneousCore/CUDAServices/interface/CUDAService.h"
55

6-
CUDAProductBase::CUDAProductBase(int device, std::shared_ptr<cuda::stream_t<>> stream, std::shared_ptr<cuda::event_t> event):
7-
stream_(std::move(stream)),
8-
event_(std::move(event)),
9-
device_(device)
10-
{}
11-
126
bool CUDAProductBase::isAvailable() const {
137
// In absence of event, the product was available already at the end
148
// of produce() of the producer.

HeterogeneousCore/CUDACore/interface/CUDAScopedContext.h

+7-3
Original file line numberDiff line numberDiff line change
@@ -73,19 +73,23 @@ class CUDAScopedContext {
7373

7474
template <typename T>
7575
std::unique_ptr<CUDAProduct<T> > wrap(T data) {
76-
createEventIfStreamBusy();
7776
// make_unique doesn't work because of private constructor
7877
//
7978
// CUDAProduct<T> constructor records CUDA event to the CUDA
8079
// stream. The event will become "occurred" after all work queued
8180
// to the stream before this point has been finished.
82-
return std::unique_ptr<CUDAProduct<T> >(new CUDAProduct<T>(device(), streamPtr(), event_, std::move(data)));
81+
std::unique_ptr<CUDAProduct<T> > ret(new CUDAProduct<T>(device(), streamPtr(), std::move(data)));
82+
createEventIfStreamBusy();
83+
ret->setEvent(event_);
84+
return ret;
8385
}
8486

8587
template <typename T, typename... Args>
8688
auto emplace(edm::Event& iEvent, edm::EDPutTokenT<T> token, Args&&... args) {
89+
auto ret = iEvent.emplace(token, device(), streamPtr(), std::forward<Args>(args)...);
8790
createEventIfStreamBusy();
88-
return iEvent.emplace(token, device(), streamPtr(), event_, std::forward<Args>(args)...);
91+
const_cast<T&>(*ret).setEvent(event_);
92+
return ret;
8993
}
9094

9195
private:

0 commit comments

Comments
 (0)