Skip to content

Commit 7618354

Browse files
committed
Improvements based on review feedback
- explicitly delete compiler generated routines - improved RAII - avoid unnecessary temporary - add arena isolations
1 parent d603932 commit 7618354

File tree

4 files changed

+22
-11
lines changed

4 files changed

+22
-11
lines changed

DataFormats/Common/interface/RefCoreStreamer.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,10 @@ namespace edm {
3838
public:
3939
RefCoreStreamerGuard(EDProductGetter const* ep) { setRefCoreStreamer(ep); }
4040
~RefCoreStreamerGuard() { unsetRefCoreStreamer(); }
41+
RefCoreStreamerGuard(RefCoreStreamerGuard const&) = delete;
42+
RefCoreStreamerGuard& operator=(RefCoreStreamerGuard const&) = delete;
43+
RefCoreStreamerGuard(RefCoreStreamerGuard&&) = delete;
44+
RefCoreStreamerGuard& operator=(RefCoreStreamerGuard&&) = delete;
4145

4246
private:
4347
static void unsetRefCoreStreamer();

IOPool/Input/src/InputFile.h

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -57,19 +57,22 @@ namespace edm {
5757

5858
class CacheGuard {
5959
public:
60-
CacheGuard(TFile* file, TObject* tree) : file_(file), tree_(tree) {}
60+
CacheGuard(TFile* file, TObject* tree, TFileCacheRead* tfcr) : file_(file), tree_(tree) {
61+
file_->SetCacheRead(tfcr, tree_, TFile::kDoNotDisconnect);
62+
}
6163
CacheGuard() = delete;
6264
CacheGuard(CacheGuard const&) = delete;
6365
CacheGuard& operator=(CacheGuard const&) = delete;
66+
CacheGuard(CacheGuard&&) = delete;
67+
CacheGuard& operator=(CacheGuard&&) = delete;
6468
~CacheGuard() { file_->SetCacheRead(nullptr, tree_, TFile::kDoNotDisconnect); }
6569

6670
private:
6771
TFile* file_;
6872
TObject* tree_;
6973
};
7074
[[nodiscard]] CacheGuard setCacheReadTemporarily(TFileCacheRead* tfcr, TObject* iTree) {
71-
file_->SetCacheRead(tfcr, iTree, TFile::kDoNotDisconnect);
72-
return CacheGuard(file_.get(), iTree);
75+
return CacheGuard(file_.get(), iTree, tfcr);
7376
}
7477
void clearCacheRead(TObject* iTree) { file_->SetCacheRead(nullptr, iTree, TFile::kDoNotDisconnect); }
7578
void logFileAction(char const* msg, char const* fileName) const;

IOPool/Input/src/RootPromptReadDelayedReader.h

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,10 @@ namespace edm {
4242

4343
~RootPromptReadDelayedReader() override;
4444

45-
RootPromptReadDelayedReader(RootPromptReadDelayedReader const&) = delete; // Disallow copying and moving
46-
RootPromptReadDelayedReader& operator=(RootPromptReadDelayedReader const&) = delete; // Disallow copying and moving
45+
RootPromptReadDelayedReader(RootPromptReadDelayedReader const&) = delete;
46+
RootPromptReadDelayedReader& operator=(RootPromptReadDelayedReader const&) = delete;
47+
RootPromptReadDelayedReader(RootPromptReadDelayedReader&&) = delete;
48+
RootPromptReadDelayedReader& operator=(RootPromptReadDelayedReader&&) = delete;
4749

4850
struct Cache {
4951
std::unique_ptr<edm::WrapperBase> wrapperBase_;
@@ -66,8 +68,8 @@ namespace edm {
6668
RootTree const& tree_;
6769
edm::propagate_const<std::shared_ptr<InputFile>> filePtr_;
6870
edm::propagate_const<DelayedReader*> nextReader_;
69-
std::unique_ptr<SharedResourcesAcquirer>
70-
resourceAcquirer_; // We do not use propagate_const because the acquirer is itself mutable.
71+
// We do not use propagate_const because the acquirer is itself mutable.
72+
std::unique_ptr<SharedResourcesAcquirer> resourceAcquirer_;
7173
std::shared_ptr<std::recursive_mutex> mutex_;
7274
InputType inputType_;
7375

IOPool/Input/src/RootTree.cc

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
#include "TTreeCache.h"
1111
#include "TLeaf.h"
1212

13+
#include "oneapi/tbb/task_arena.h"
1314
#include <cassert>
1415

1516
namespace edm {
@@ -176,8 +177,7 @@ namespace edm {
176177
std::unique_ptr<WrapperBase> roottree::BranchInfo::newWrapper() const {
177178
assert(nullptr != classCache_);
178179
void* p = classCache_->New();
179-
std::unique_ptr<WrapperBase> edp = getWrapperBasePtr(p, offsetToWrapperBase_);
180-
return edp;
180+
return getWrapperBasePtr(p, offsetToWrapperBase_);
181181
}
182182

183183
void RootTree::addBranch(ProductDescription const& prod, std::string const& oldBranchName) {
@@ -426,8 +426,10 @@ namespace edm {
426426
}
427427

428428
void RootTree::getEntryForAllBranches() const {
429-
auto guard = filePtr_->setCacheReadTemporarily(rawTreeCache_.get(), tree_);
430-
tree_->GetEntry(entryNumber_);
429+
oneapi::tbb::this_task_arena::isolate([&]() {
430+
auto guard = filePtr_->setCacheReadTemporarily(treeCache_.get(), tree_);
431+
tree_->GetEntry(entryNumber_);
432+
});
431433
}
432434

433435
void RootTree::getEntry(TBranch* branch, EntryNumber entryNumber) const {

0 commit comments

Comments
 (0)