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

Use shared_ptr for MutableMonitorElementData [12_0] #35757

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 6 additions & 6 deletions DQMServices/Core/interface/MonitorElement.h
Expand Up @@ -26,6 +26,7 @@
#include "TAxis.h"

#include <mutex>
#include <memory>
#include <string>
#include <atomic>
#include <sstream>
Expand Down Expand Up @@ -136,9 +137,8 @@ namespace dqm::impl {
};

private:
MutableMonitorElementData *mutable_; // only set if this is a mutable copy of this ME
std::shared_ptr<MutableMonitorElementData> mutable_; // only set if this is a mutable copy of this ME
// there are no immutable MEs at this time, but we might need them in the future.
bool is_owned_; // true if we are responsible for deleting the mutable object.
/**
* To do anything to the MEs data, one needs to obtain an access object.
* This object will contain the lock guard if one is needed. We differentiate
Expand Down Expand Up @@ -176,21 +176,21 @@ namespace dqm::impl {
// new ME. The new ME will own this data.
MonitorElement(MonitorElementData &&data);
// Create new ME and take ownership of this data.
MonitorElement(MutableMonitorElementData *data);
MonitorElement(std::shared_ptr<MutableMonitorElementData> data);
// Create a new ME sharing data with this existing ME.
MonitorElement(MonitorElement *me);

// return a new clone of the data of this ME. Calls ->Clone(), new object
// is owned by the returned value.
MonitorElementData cloneMEData();

// Remove access and ownership to the data. The flag is used for a sanity check.
MutableMonitorElementData *release(bool expectOwned);
// Remove access to the data.
std::shared_ptr<MutableMonitorElementData> release();

// re-initialize this ME as a shared copy of the other.
void switchData(MonitorElement *other);
// re-initialize taking ownership of this data.
void switchData(MutableMonitorElementData *data);
void switchData(std::shared_ptr<MutableMonitorElementData> data);

// Replace the ROOT object in this ME's data with the new object, taking
// ownership. The old object is deleted.
Expand Down
8 changes: 4 additions & 4 deletions DQMServices/Core/src/DQMStore.cc
Expand Up @@ -338,7 +338,7 @@ namespace dqm::implementation {
MonitorElement* oldme = *proto;
assert(oldme->getScope() == key.scope_);
prototypes.erase(proto);
auto medata = oldme->release(/* expectOwned */ true); // destroy the ME, get its data.
auto medata = oldme->release(); // destroy the ME, get its data.
// in this situation, nobody should be filling the ME concurrently.
medata->data_.key_.id_ = key.id_;
// We reuse the ME object here, even if we don't have to. This ensures
Expand Down Expand Up @@ -408,7 +408,7 @@ namespace dqm::implementation {
// reuse that.
MonitorElement* oldme = *proto;
prototypes.erase(proto);
auto medata = oldme->release(/* expectOwned */ true); // destroy the ME, get its data.
auto medata = oldme->release(); // destroy the ME, get its data.
// in this situation, nobody should be filling the ME concurrently.
medata->data_.key_.id_ = edm::LuminosityBlockID(run, lumi);
// We reuse the ME object here, even if we don't have to. This ensures
Expand Down Expand Up @@ -518,7 +518,7 @@ namespace dqm::implementation {
if (me->isValid() && checkScope(me->getScope()) == true) {
// if we left the scope, simply release the data.
debugTrackME("leaveLumi (release)", me, nullptr);
me->release(/* expectOwned */ false);
me->release();
}
}
}
Expand Down Expand Up @@ -576,7 +576,7 @@ namespace dqm::implementation {
meset.clear();

for (MonitorElement* me : torecycle) {
auto medata = me->release(/* expectOwned */ true); // destroy the ME, get its data.
auto medata = me->release(); // destroy the ME, get its data.
medata->data_.key_.id_ = edm::LuminosityBlockID(); // prototype
// We reuse the ME object here, even if we don't have to. This ensures
// that when running single-threaded without concurrent lumis/runs,
Expand Down
25 changes: 8 additions & 17 deletions DQMServices/Core/src/MonitorElement.cc
Expand Up @@ -35,12 +35,11 @@ namespace dqm::impl {
}

MonitorElement::MonitorElement(MonitorElementData &&data) {
this->mutable_ = new MutableMonitorElementData();
this->mutable_ = std::make_shared<MutableMonitorElementData>();
this->mutable_->data_ = std::move(data);
this->is_owned_ = true;
syncCoreObject();
}
MonitorElement::MonitorElement(MutableMonitorElementData *data) { switchData(data); }
MonitorElement::MonitorElement(std::shared_ptr<MutableMonitorElementData> data) { switchData(std::move(data)); }
MonitorElement::MonitorElement(MonitorElement *me) { switchData(me); }

MonitorElementData MonitorElement::cloneMEData() {
Expand All @@ -54,25 +53,20 @@ namespace dqm::impl {
return out;
}

MutableMonitorElementData *MonitorElement::release(bool expectOwned) {
assert(this->is_owned_ == expectOwned);
MutableMonitorElementData *data = this->mutable_;
this->mutable_ = nullptr;
this->is_owned_ = false;
assert(!expectOwned || data);
std::shared_ptr<MutableMonitorElementData> MonitorElement::release() {
auto data = this->mutable_;
this->mutable_.reset();
return data;
}

void MonitorElement::switchData(MonitorElement *other) {
assert(other);
this->mutable_ = other->mutable_;
this->is_owned_ = false;
syncCoreObject();
}

void MonitorElement::switchData(MutableMonitorElementData *data) {
this->mutable_ = data;
this->is_owned_ = true;
void MonitorElement::switchData(std::shared_ptr<MutableMonitorElementData> data) {
this->mutable_ = std::move(data);
syncCoreObject();
}

Expand Down Expand Up @@ -150,10 +144,7 @@ namespace dqm::impl {
}
}

MonitorElement::~MonitorElement() {
if (is_owned_)
delete mutable_;
}
MonitorElement::~MonitorElement() {}

//utility function to check the consistency of the axis labels
//taken from TH1::CheckBinLabels which is not public
Expand Down