From 5a39c08f3f91d2e1a690fb5a7aec887720fec127 Mon Sep 17 00:00:00 2001 From: Peter van Gemmeren <gemmeren@anl.gov> Date: Thu, 24 Sep 2020 18:24:40 -0500 Subject: [PATCH 1/4] Remove calls to TTree::GetBranch(), where they have become much more costly in ROOT 6.22 --- .../src/RootTreeIndexContainer.cpp | 30 +++++++++---------- .../src/RootTreeIndexContainer.h | 3 +- 2 files changed, 16 insertions(+), 17 deletions(-) diff --git a/Database/APR/RootStorageSvc/src/RootTreeIndexContainer.cpp b/Database/APR/RootStorageSvc/src/RootTreeIndexContainer.cpp index eea0dc2bdd28..91f323120bca 100644 --- a/Database/APR/RootStorageSvc/src/RootTreeIndexContainer.cpp +++ b/Database/APR/RootStorageSvc/src/RootTreeIndexContainer.cpp @@ -18,7 +18,7 @@ using namespace pool; using namespace std; -RootTreeIndexContainer::RootTreeIndexContainer() : RootTreeContainer(), m_index_ref(nullptr), m_index_entries(0), m_index_multi(0), m_index(nullptr) { +RootTreeIndexContainer::RootTreeIndexContainer() : RootTreeContainer(), m_index_ref(nullptr), m_index_shr(nullptr), m_index_entries(0), m_index_multi(0), m_index(nullptr) { m_index = new long long int; m_index_multi = getpid(); } @@ -32,38 +32,36 @@ RootTreeIndexContainer::~RootTreeIndexContainer() { long long int RootTreeIndexContainer::nextRecordId() { long long int s = m_index_multi; s = s << 32; - if (m_tree->GetBranch("index_ref") != nullptr) { - s += m_index_entries; - } else { + if (m_index_ref == nullptr && m_index_shr == nullptr) { s += RootTreeContainer::nextRecordId(); + } else { + s += m_index_entries; } return s; } DbStatus RootTreeIndexContainer::writeObject(ActionList::value_type& action) { - long long int s = 0; - if (isBranchContainer()) { - TBranch * pBranch = m_tree->GetBranch(m_branchName.c_str()); - if (pBranch != nullptr) s = pBranch->GetEntries(); - } else { - s = m_tree->GetEntries(); - } - if (m_index_ref == nullptr && m_tree->GetBranch("index_ref") == nullptr) { - m_index_ref = (TBranch*)m_tree->Branch("index_ref", m_index); + // If ROOT TTree has index_ref TBranch share it, otherwise create one and make it part of this POOL container + if (m_index_ref == nullptr && m_index_shr == nullptr) { + m_index_shr = m_tree->GetBranch("index_ref"); + if (m_index_shr == nullptr) { + m_index_ref = static_cast<TBranch*>(m_tree->Branch("index_ref", m_index)); + } } - if (m_index_ref != nullptr && s >= m_index_ref->GetEntries()) { + // POOL container that owns the index_ref TBranch fills it. + if (m_index_ref != nullptr && m_index_entries >= m_index_ref->GetEntries()) { *m_index = this->nextRecordId(); m_index_ref->SetAddress(m_index); if (isBranchContainer() && !m_treeFillMode) m_index_ref->Fill(); } if (isBranchContainer() && !m_treeFillMode) { - m_tree->SetEntries(s); + m_tree->SetEntries(m_index_entries); m_index_entries++; } DbStatus status = RootTreeContainer::writeObject(action); if (isBranchContainer() && !m_treeFillMode) { - m_tree->SetEntries(s + 1); + m_tree->SetEntries(m_index_entries); } else { m_index_entries++; } diff --git a/Database/APR/RootStorageSvc/src/RootTreeIndexContainer.h b/Database/APR/RootStorageSvc/src/RootTreeIndexContainer.h index 158839e8223a..1303bd9f1661 100644 --- a/Database/APR/RootStorageSvc/src/RootTreeIndexContainer.h +++ b/Database/APR/RootStorageSvc/src/RootTreeIndexContainer.h @@ -59,8 +59,9 @@ namespace pool { virtual DbStatus transAct(Transaction::Action action); private: - /// Pointer to index branch + /// Pointer to index branch (ref owns the branch, shr doesn't) TBranch* m_index_ref; + TBranch* m_index_shr; long long int m_index_entries; /// Index multiplier (e.g. pid - ppid), fill in c'tor int m_index_multi; -- GitLab From 38144c1b6956bf9f484b29252ed1dfe7dfc7ed50 Mon Sep 17 00:00:00 2001 From: Peter van Gemmeren <gemmeren@anl.gov> Date: Thu, 24 Sep 2020 19:23:32 -0500 Subject: [PATCH 2/4] For setting enrty number on TTrees, need to get real TBranch entry, in case TMemFile resets it. --- Database/APR/RootStorageSvc/src/RootTreeIndexContainer.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/Database/APR/RootStorageSvc/src/RootTreeIndexContainer.cpp b/Database/APR/RootStorageSvc/src/RootTreeIndexContainer.cpp index 91f323120bca..d75e533cfe84 100644 --- a/Database/APR/RootStorageSvc/src/RootTreeIndexContainer.cpp +++ b/Database/APR/RootStorageSvc/src/RootTreeIndexContainer.cpp @@ -55,13 +55,16 @@ DbStatus RootTreeIndexContainer::writeObject(ActionList::value_type& action) { m_index_ref->SetAddress(m_index); if (isBranchContainer() && !m_treeFillMode) m_index_ref->Fill(); } + long long int s = m_index_entries; + TBranch* pBranch = m_tree->GetBranch(m_branchName.c_str()); + if (pBranch != nullptr) s = pBranch->GetEntries(); if (isBranchContainer() && !m_treeFillMode) { - m_tree->SetEntries(m_index_entries); + m_tree->SetEntries(s); m_index_entries++; } DbStatus status = RootTreeContainer::writeObject(action); if (isBranchContainer() && !m_treeFillMode) { - m_tree->SetEntries(m_index_entries); + m_tree->SetEntries(s); } else { m_index_entries++; } -- GitLab From 786aa141921ffee9eaee2a3622134d0dd3342818 Mon Sep 17 00:00:00 2001 From: Peter van Gemmeren <gemmeren@anl.gov> Date: Thu, 24 Sep 2020 19:28:00 -0500 Subject: [PATCH 3/4] Fix typo --- Database/APR/RootStorageSvc/src/RootTreeIndexContainer.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Database/APR/RootStorageSvc/src/RootTreeIndexContainer.cpp b/Database/APR/RootStorageSvc/src/RootTreeIndexContainer.cpp index d75e533cfe84..88603796402a 100644 --- a/Database/APR/RootStorageSvc/src/RootTreeIndexContainer.cpp +++ b/Database/APR/RootStorageSvc/src/RootTreeIndexContainer.cpp @@ -64,7 +64,7 @@ DbStatus RootTreeIndexContainer::writeObject(ActionList::value_type& action) { } DbStatus status = RootTreeContainer::writeObject(action); if (isBranchContainer() && !m_treeFillMode) { - m_tree->SetEntries(s); + m_tree->SetEntries(s + 1); } else { m_index_entries++; } -- GitLab From e89e92cb97a3aab3d255257a612425a66b93e3ad Mon Sep 17 00:00:00 2001 From: Peter van Gemmeren <gemmeren@anl.gov> Date: Thu, 24 Sep 2020 21:27:14 -0500 Subject: [PATCH 4/4] Get rid of the last GetBranch() --- .../RootStorageSvc/src/RootTreeIndexContainer.cpp | 12 ++++++------ .../APR/RootStorageSvc/src/RootTreeIndexContainer.h | 1 + 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/Database/APR/RootStorageSvc/src/RootTreeIndexContainer.cpp b/Database/APR/RootStorageSvc/src/RootTreeIndexContainer.cpp index 88603796402a..509786e17916 100644 --- a/Database/APR/RootStorageSvc/src/RootTreeIndexContainer.cpp +++ b/Database/APR/RootStorageSvc/src/RootTreeIndexContainer.cpp @@ -18,7 +18,7 @@ using namespace pool; using namespace std; -RootTreeIndexContainer::RootTreeIndexContainer() : RootTreeContainer(), m_index_ref(nullptr), m_index_shr(nullptr), m_index_entries(0), m_index_multi(0), m_index(nullptr) { +RootTreeIndexContainer::RootTreeIndexContainer() : RootTreeContainer(), m_index_ref(nullptr), m_index_shr(nullptr), m_index_entries(0), m_ttree_entries(0), m_index_multi(0), m_index(nullptr) { m_index = new long long int; m_index_multi = getpid(); } @@ -55,18 +55,17 @@ DbStatus RootTreeIndexContainer::writeObject(ActionList::value_type& action) { m_index_ref->SetAddress(m_index); if (isBranchContainer() && !m_treeFillMode) m_index_ref->Fill(); } - long long int s = m_index_entries; - TBranch* pBranch = m_tree->GetBranch(m_branchName.c_str()); - if (pBranch != nullptr) s = pBranch->GetEntries(); if (isBranchContainer() && !m_treeFillMode) { - m_tree->SetEntries(s); + m_tree->SetEntries(m_ttree_entries); m_index_entries++; + m_ttree_entries++; } DbStatus status = RootTreeContainer::writeObject(action); if (isBranchContainer() && !m_treeFillMode) { - m_tree->SetEntries(s + 1); + m_tree->SetEntries(m_ttree_entries); } else { m_index_entries++; + m_ttree_entries++; } return status; } @@ -75,6 +74,7 @@ DbStatus RootTreeIndexContainer::writeObject(ActionList::value_type& action) { DbStatus RootTreeIndexContainer::transAct(Transaction::Action action) { DbStatus status = RootTreeContainer::transAct(action); if (action == Transaction::TRANSACT_FLUSH) { + m_ttree_entries = 0; if (m_tree == nullptr) return Error; if (m_index_ref != nullptr && m_tree->GetEntries() > 0 && dynamic_cast<TMemFile*>(m_tree->GetCurrentFile()) == nullptr && m_tree->GetEntryNumberWithIndex(nextRecordId()) == -1) { m_tree->BuildIndex("index_ref"); diff --git a/Database/APR/RootStorageSvc/src/RootTreeIndexContainer.h b/Database/APR/RootStorageSvc/src/RootTreeIndexContainer.h index 1303bd9f1661..13cc88ed2a8b 100644 --- a/Database/APR/RootStorageSvc/src/RootTreeIndexContainer.h +++ b/Database/APR/RootStorageSvc/src/RootTreeIndexContainer.h @@ -63,6 +63,7 @@ namespace pool { TBranch* m_index_ref; TBranch* m_index_shr; long long int m_index_entries; + long long int m_ttree_entries; /// Index multiplier (e.g. pid - ppid), fill in c'tor int m_index_multi; /// Index (64 bit) -- GitLab