Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement event batching in the miniframework #143

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
149 changes: 143 additions & 6 deletions src/cuda/Framework/EDProducer.h
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
#ifndef EDProducerBase_h
#define EDProducerBase_h

#include <memory>

#include "Framework/EventRange.h"
#include "Framework/WaitingTaskWithArenaHolder.h"

namespace edm {
Expand All @@ -14,9 +17,13 @@ namespace edm {

bool hasAcquire() const { return false; }

void doAcquire(Event const& event, EventSetup const& eventSetup, WaitingTaskWithArenaHolder holder) {}
void doAcquire(ConstEventRange events, EventSetup const& eventSetup, WaitingTaskWithArenaHolder holder) {}

void doProduce(Event& event, EventSetup const& eventSetup) { produce(event, eventSetup); }
void doProduce(EventRange events, EventSetup const& eventSetup) {
for (Event& event : events) {
produce(event, eventSetup);
}
}

virtual void produce(Event& event, EventSetup const& eventSetup) = 0;

Expand All @@ -27,27 +34,157 @@ namespace edm {
private:
};

class EDBatchingProducer {
public:
EDBatchingProducer() = default;
virtual ~EDBatchingProducer() = default;

bool hasAcquire() const { return false; }

void doAcquire(ConstEventRange events, EventSetup const& eventSetup, WaitingTaskWithArenaHolder holder) {}

void doProduce(EventRange events, EventSetup const& eventSetup) { produce(events, eventSetup); }

virtual void produce(EventRange events, EventSetup const& eventSetup) = 0;

void doEndJob() { endJob(); }

virtual void endJob() {}

private:
};

template <typename T = void>
class EDProducerExternalWork {
public:
using AsyncState = T;

EDProducerExternalWork() = default;
virtual ~EDProducerExternalWork() = default;

bool hasAcquire() const { return true; }

void doAcquire(Event const& event, EventSetup const& eventSetup, WaitingTaskWithArenaHolder holder) {
acquire(event, eventSetup, std::move(holder));
void doAcquire(ConstEventRange events, EventSetup const& eventSetup, WaitingTaskWithArenaHolder holder) {
if (events.size() > statesSize_) {
statesSize_ = events.size();
states_ = std::make_unique<AsyncState[]>(statesSize_);
}
for (size_t i = 0; i < events.size(); ++i) {
acquire(events[i], eventSetup, holder, states_[i]);
}
}

void doProduce(Event& event, EventSetup const& eventSetup) { produce(event, eventSetup); }
virtual void acquire(Event const& event,
EventSetup const& eventSetup,
WaitingTaskWithArenaHolder holder,
AsyncState& state) const = 0;

void doProduce(EventRange events, EventSetup const& eventSetup) {
for (size_t i = 0; i < events.size(); ++i) {
produce(events[i], eventSetup, states_[i]);
}
}

virtual void produce(Event& event, EventSetup const& eventSetup, AsyncState& state) = 0;

void doEndJob() { endJob(); }

virtual void endJob() {}

private:
size_t statesSize_ = 0;
std::unique_ptr<AsyncState[]> states_;
};

template <>
class EDProducerExternalWork<void> {
public:
EDProducerExternalWork() = default;
virtual ~EDProducerExternalWork() = default;

bool hasAcquire() const { return true; }

void doAcquire(ConstEventRange events, EventSetup const& eventSetup, WaitingTaskWithArenaHolder holder) {
for (Event const& event : events) {
acquire(event, eventSetup, holder);
}
}

virtual void acquire(Event const& event, EventSetup const& eventSetup, WaitingTaskWithArenaHolder holder) const = 0;

void doProduce(EventRange events, EventSetup const& eventSetup) {
for (Event& event : events) {
produce(event, eventSetup);
}
}

virtual void acquire(Event const& event, EventSetup const& eventSetup, WaitingTaskWithArenaHolder holder) = 0;
virtual void produce(Event& event, EventSetup const& eventSetup) = 0;

void doEndJob() { endJob(); }

virtual void endJob() {}

private:
};

template <typename T = void>
class EDBatchingProducerExternalWork {
public:
using AsyncState = T;

EDBatchingProducerExternalWork() = default;
virtual ~EDBatchingProducerExternalWork() = default;

bool hasAcquire() const { return true; }

void doAcquire(ConstEventRange events, EventSetup const& eventSetup, WaitingTaskWithArenaHolder holder) {
acquire(events, eventSetup, holder, state_);
}

virtual void acquire(ConstEventRange events,
EventSetup const& eventSetup,
WaitingTaskWithArenaHolder holder,
AsyncState& state) const = 0;

void doProduce(EventRange events, EventSetup const& eventSetup) { produce(events, eventSetup, state_); }

virtual void produce(EventRange events, EventSetup const& eventSetup, AsyncState& states) = 0;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really minor, but is this a typo?

Suggested change
virtual void produce(EventRange events, EventSetup const& eventSetup, AsyncState& states) = 0;
virtual void produce(EventRange events, EventSetup const& eventSetup, AsyncState& state) = 0;

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've used both spellings while prototyping, so I don't mind either way: this should capture the "state" or "states" of multiple events:

  • state is more consistent with AsyncState
  • states is more consistent with events

Re-thinking a bit more... maybe state is more correct, since it is one object, not e.g. a vector<AsyncState>.


void doEndJob() { endJob(); }

virtual void endJob() {}

private:
AsyncState state_;
};

template <>
class EDBatchingProducerExternalWork<void> {
public:
EDBatchingProducerExternalWork() = default;
virtual ~EDBatchingProducerExternalWork() = default;

bool hasAcquire() const { return true; }

void doAcquire(ConstEventRange events, EventSetup const& eventSetup, WaitingTaskWithArenaHolder holder) {
acquire(events, eventSetup, holder);
}

virtual void acquire(ConstEventRange events,
EventSetup const& eventSetup,
WaitingTaskWithArenaHolder holder) const = 0;

void doProduce(EventRange events, EventSetup const& eventSetup) { produce(events, eventSetup); }

virtual void produce(EventRange events, EventSetup const& eventSetup) = 0;

void doEndJob() { endJob(); }

virtual void endJob() {}

private:
};

} // namespace edm

#endif
36 changes: 36 additions & 0 deletions src/cuda/Framework/EventBatch.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
#ifndef EventBatch_h
#define EventBatch_h

#include <vector>

#include "Framework/Event.h"
#include "Framework/EventRange.h"

namespace edm {

class EventBatch {
public:
Event& emplace(int streamId, int eventId, ProductRegistry const& reg) {
events_.emplace_back(streamId, eventId, reg);
return events_.back();
}

void reserve(size_t capacity) { events_.reserve(capacity); }

void clear() { events_.clear(); }

size_t size() const { return events_.size(); }

bool empty() const { return events_.empty(); }

EventRange range() { return EventRange(&events_.front(), &events_.back() + 1); }

ConstEventRange range() const { return ConstEventRange(&events_.front(), &events_.back() + 1); }
Comment on lines +26 to +28
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not very relevant for prototyping itself, but i think being able to use something along

Suggested change
EventRange range() { return EventRange(&events_.front(), &events_.back() + 1); }
ConstEventRange range() const { return ConstEventRange(&events_.front(), &events_.back() + 1); }
EventRange range() { return EventRange(events_.begin(), events_.end()); }
ConstEventRange range() const { return ConstEventRange(events_.begin(), events_.end()); }

would be clearer to read.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason I didn't do that was to avoid making EventRange and ConstEventRange use std::vector<Event>::iterator and const_iterator: then it could only work for a range of events stored in a vector, while using plain pointers it can work with any consecutive range of events.

An alternative could be to add a constructor like

template <typename IT>
EventRange(IT begin, IT end) : begin_(std::addressof(*begin)), end_(std::addressof((*end)) { ... }

but then one could be tempted to pass non-contiguous iterators.
Maybe one could add something like

  assert((end - begin) == (end_ - begin_));

or, once we move to C++20 with concepts, we could use contiguous_iterator ?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the purposes of the prototype the current pointers are fine for me. With C++20 maybe even passing the range as std::span could be an option.


private:
std::vector<edm::Event> events_;
};

} // namespace edm

#endif // EventBatch_h
96 changes: 96 additions & 0 deletions src/cuda/Framework/EventRange.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
#ifndef EventRange_h
#define EventRange_h

#include <cassert>
#include <sstream>
#include <stdexcept>

#include "Framework/Event.h"

namespace edm {

class EventRange {
public:
EventRange(Event* begin, Event* end) : begin_(begin), end_(end) {
assert(begin_);
assert(end_);
assert(end_ >= begin_);
}

Event* begin() { return begin_; }

Event const* begin() const { return begin_; }

Event* end() { return end_; }

Event const* end() const { return end_; }

size_t size() const { return end_ - begin_; }

bool empty() const { return end_ == begin_; }

Event& at(size_t index) {
if (index >= size()) {
std::stringstream msg;
msg << "EventRange::at() range check failed: index " << index << " is outside the range [0.." << size() << ")";
throw std::out_of_range(msg.str());
}
return begin_[index];
}

Event const& at(size_t index) const {
if (index >= size()) {
std::stringstream msg;
msg << "EventRange::at() range check failed: index " << index << " is outside the range [0.." << size() << ")";
throw std::out_of_range(msg.str());
}
return begin_[index];
}

Event& operator[](size_t index) { return begin_[index]; }

Event const& operator[](size_t index) const { return begin_[index]; }

private:
Event* begin_;
Event* end_;
};

class ConstEventRange {
public:
ConstEventRange(Event const* begin, Event const* end) : begin_(begin), end_(end) {
assert(begin_);
assert(end_);
assert(end_ >= begin_);
}

ConstEventRange(EventRange range) : begin_(range.begin()), end_(range.end()) {}

Event const* begin() const { return begin_; }

Event const* end() const { return end_; }

size_t size() const { return end_ - begin_; }

bool empty() const { return end_ == begin_; }

Event const& at(size_t index) {
if (index >= size()) {
std::stringstream msg;
msg << "ConstEventRange::at() range check failed: index " << index << " is outside the range [0.." << size()
<< ")";
throw std::out_of_range(msg.str());
}
return begin_[index];
}

Event const& operator[](size_t index) const { return begin_[index]; }

private:
Event const* begin_;
Event const* end_;
};

} // namespace edm

#endif // EventRange_h
14 changes: 9 additions & 5 deletions src/cuda/Framework/WaitingTaskHolder.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ namespace edm {
explicit WaitingTaskHolder(edm::WaitingTask* iTask) : m_task(iTask) { m_task->increment_ref_count(); }
~WaitingTaskHolder() {
if (m_task) {
doneWaiting(std::exception_ptr{});
doneWaiting();
}
}

Expand Down Expand Up @@ -66,10 +66,7 @@ namespace edm {
}
}

void doneWaiting(std::exception_ptr iExcept) {
if (iExcept) {
m_task->dependentTaskFailed(iExcept);
}
void doneWaiting() {
//spawn can run the task before we finish
// doneWaiting and some other thread might
// try to reuse this object. Resetting
Expand All @@ -81,6 +78,13 @@ namespace edm {
}
}

void doneWaiting(std::exception_ptr iExcept) {
if (iExcept) {
m_task->dependentTaskFailed(iExcept);
}
doneWaiting();
}

private:
// ---------- member data --------------------------------
WaitingTask* m_task;
Expand Down
18 changes: 13 additions & 5 deletions src/cuda/Framework/WaitingTaskWithArenaHolder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ namespace edm {

WaitingTaskWithArenaHolder::~WaitingTaskWithArenaHolder() {
if (m_task) {
doneWaiting(std::exception_ptr{});
doneWaiting();
}
}

Expand Down Expand Up @@ -58,10 +58,7 @@ namespace edm {
// into the correct arena of threads. Use of the arena allows doneWaiting
// to be called from a thread outside the arena of threads that will manage
// the task. doneWaiting can be called from a non-TBB thread.
void WaitingTaskWithArenaHolder::doneWaiting(std::exception_ptr iExcept) {
if (iExcept) {
m_task->dependentTaskFailed(iExcept);
}
void WaitingTaskWithArenaHolder::doneWaiting() {
//enqueue can run the task before we finish
// doneWaiting and some other thread might
// try to reuse this object. Resetting
Expand All @@ -75,6 +72,17 @@ namespace edm {
}
}

// This spawns the task. The arena is needed to get the task spawned
// into the correct arena of threads. Use of the arena allows doneWaiting
// to be called from a thread outside the arena of threads that will manage
// the task. doneWaiting can be called from a non-TBB thread.
void WaitingTaskWithArenaHolder::doneWaiting(std::exception_ptr iExcept) {
if (iExcept) {
m_task->dependentTaskFailed(iExcept);
}
doneWaiting();
}

// This next function is useful if you know from the context that
// m_arena (which is set when the constructor was executes) is the
// same arena in which you want to execute the doneWaiting function.
Expand Down
Loading