From d3f8562773b9f06dab973aa18dd5c14a0663fea2 Mon Sep 17 00:00:00 2001 From: Juan Miguel Carceller <22276694+jmcarcell@users.noreply.github.com> Date: Tue, 15 Oct 2024 18:16:03 +0200 Subject: [PATCH] Use bit manipulation with the UniqueIDGenSvc and add a check for repeated numbers (#247) Co-authored-by: Thomas Madlener --- k4FWCore/components/UniqueIDGenSvc.cpp | 58 ++++++++++++++----- k4FWCore/components/UniqueIDGenSvc.h | 16 +++-- .../include/k4Interface/IUniqueIDGenSvc.h | 3 - test/k4FWCoreTest/CMakeLists.txt | 3 +- .../src/components/TestUniqueIDGenSvc.cpp | 17 ++++-- .../src/components/TestUniqueIDGenSvc.h | 6 +- 6 files changed, 69 insertions(+), 34 deletions(-) diff --git a/k4FWCore/components/UniqueIDGenSvc.cpp b/k4FWCore/components/UniqueIDGenSvc.cpp index 5f6f0f7b..55ff6abd 100644 --- a/k4FWCore/components/UniqueIDGenSvc.cpp +++ b/k4FWCore/components/UniqueIDGenSvc.cpp @@ -18,27 +18,53 @@ */ #include "UniqueIDGenSvc.h" -DECLARE_COMPONENT(UniqueIDGenSvc) +#include +#include +#include UniqueIDGenSvc::UniqueIDGenSvc(const std::string& name, ISvcLocator* svcLoc) : base_class(name, svcLoc) {} -StatusCode UniqueIDGenSvc::initialize() { - StatusCode sc = Service::initialize(); - return sc; -} - -const size_t bits32 = std::numeric_limits::digits; -const size_t bits64 = std::numeric_limits::digits; -const size_t bitsSizeT = std::numeric_limits::digits; +constexpr size_t bits32 = std::numeric_limits::digits; +constexpr size_t bits64 = std::numeric_limits::digits; +constexpr size_t bitsSizeT = std::numeric_limits::digits; size_t UniqueIDGenSvc::getUniqueID(uint32_t evt_num, uint32_t run_num, const std::string& name) const { - std::bitset seed_bits(this->m_seed); - std::bitset event_num_bits(evt_num), run_num_bits(run_num); - size_t str_hash = std::hash{}(name); - std::bitset name_bits(str_hash); + std::bitset seed_bits = this->m_seed.value(); + std::bitset event_num_bits = evt_num, run_num_bits = run_num; + size_t str_hash = std::hash{}(name); + std::bitset name_bits = str_hash; + + std::bitset combined_bits; - std::bitset combined_bits(seed_bits.to_string() + event_num_bits.to_string() + - run_num_bits.to_string() + name_bits.to_string()); + for (size_t i = 0; i < bitsSizeT; i++) { + combined_bits[i] = name_bits[i]; + } + for (size_t i = 0; i < bits32; i++) { + combined_bits[i + bitsSizeT] = run_num_bits[i]; + } + for (size_t i = 0; i < bits32; i++) { + combined_bits[i + bits32 + bitsSizeT] = event_num_bits[i]; + } + for (size_t i = 0; i < bits64; i++) { + combined_bits[i + bits32 + bits32 + bitsSizeT] = seed_bits[i]; + } - return std::hash>{}(combined_bits); + auto hash = std::hash>{}(combined_bits); + bool inserted = false; + { + std::lock_guard lock(m_mutex); + std::tie(std::ignore, inserted) = m_uniqueIDs.insert(hash); + } + if (!inserted) { + error() << "Event number " << evt_num << ", run number " << run_num << " and algorithm name \"" << name + << "\" have already been used. Please check the uniqueness of the event number, run number and name." + << endmsg; + if (m_throwIfDuplicate) { + throw std::runtime_error("Duplicate event number, run number and algorithm name"); + } + } + + return hash; } + +DECLARE_COMPONENT(UniqueIDGenSvc) diff --git a/k4FWCore/components/UniqueIDGenSvc.h b/k4FWCore/components/UniqueIDGenSvc.h index 966fbd99..ff202370 100644 --- a/k4FWCore/components/UniqueIDGenSvc.h +++ b/k4FWCore/components/UniqueIDGenSvc.h @@ -19,11 +19,13 @@ #ifndef FWCORE_UNIQUEIDGENSVC_H #define FWCORE_UNIQUEIDGENSVC_H +#include "GaudiKernel/Service.h" +#include "k4Interface/IUniqueIDGenSvc.h" + #include +#include #include - -#include -#include "k4Interface/IUniqueIDGenSvc.h" +#include /** @class UniqueIDGenSvc * Generate unique, reproducible numbers using @@ -34,11 +36,13 @@ class UniqueIDGenSvc : public extends { public: UniqueIDGenSvc(const std::string& name, ISvcLocator* svcLoc); - StatusCode initialize() override; - size_t getUniqueID(uint32_t evt_num, uint32_t run_num, const std::string& name) const override; + size_t getUniqueID(uint32_t evt_num, uint32_t run_num, const std::string& name) const override; private: - Gaudi::Property m_seed{this, "Seed", {123456789}}; + Gaudi::Property m_seed{this, "Seed", {123456789}}; + mutable std::unordered_set m_uniqueIDs; + mutable std::mutex m_mutex; + Gaudi::Property m_throwIfDuplicate{this, "ThrowIfDuplicate", {true}}; }; #endif diff --git a/k4Interface/include/k4Interface/IUniqueIDGenSvc.h b/k4Interface/include/k4Interface/IUniqueIDGenSvc.h index c7a8bf07..913f18b4 100644 --- a/k4Interface/include/k4Interface/IUniqueIDGenSvc.h +++ b/k4Interface/include/k4Interface/IUniqueIDGenSvc.h @@ -19,10 +19,7 @@ #ifndef FWCORE_IUNIQUEIDGENSVC_H #define FWCORE_IUNIQUEIDGENSVC_H -#include #include -#include -#include #include #include diff --git a/test/k4FWCoreTest/CMakeLists.txt b/test/k4FWCoreTest/CMakeLists.txt index 2bb2cb47..6d48d076 100644 --- a/test/k4FWCoreTest/CMakeLists.txt +++ b/test/k4FWCoreTest/CMakeLists.txt @@ -125,7 +125,8 @@ add_test(NAME checkKeepDropSwitch COMMAND python scripts/check_KeepDropSwitch.py ${PROJECT_BINARY_DIR}/test/k4FWCoreTest/output_k4test_exampledata_2.root) set_test_env(checkKeepDropSwitch) set_property(TEST checkKeepDropSwitch APPEND PROPERTY DEPENDS ReadExampleEventData) -add_test_with_env(TestUniqueIDGenSvc options/TestUniqueIDGenSvc.py) +add_test_with_env(TestUniqueIDGenSvc options/TestUniqueIDGenSvc.py -n 1) +add_test_with_env(TestUniqueIDGenSvcRepeated options/TestUniqueIDGenSvc.py -n 2 PROPERTIES PASS_REGULAR_EXPRESSION "Duplicate event number, run number and algorithm name") add_test_with_env(TestEventHeaderFiller options/createEventHeader.py) add_test_with_env(EventHeaderCheck options/runEventHeaderCheck.py PROPERTIES DEPENDS TestEventHeaderFiller) add_test(NAME TestExec WORKING_DIRECTORY ${CMAKE_CURRENT_LIST_DIR} COMMAND python options/TestExec.py) diff --git a/test/k4FWCoreTest/src/components/TestUniqueIDGenSvc.cpp b/test/k4FWCoreTest/src/components/TestUniqueIDGenSvc.cpp index fb3c6add..bc39ab1b 100644 --- a/test/k4FWCoreTest/src/components/TestUniqueIDGenSvc.cpp +++ b/test/k4FWCoreTest/src/components/TestUniqueIDGenSvc.cpp @@ -18,6 +18,8 @@ */ #include "TestUniqueIDGenSvc.h" +#include + DECLARE_COMPONENT(TestUniqueIDGenSvc) TestUniqueIDGenSvc::TestUniqueIDGenSvc(const std::string& aName, ISvcLocator* aSvcLoc) @@ -33,16 +35,19 @@ StatusCode TestUniqueIDGenSvc::initialize() { return StatusCode::SUCCESS; } +// This is meant to run up to two times +// For the first event, check that when giving two different event numbers, the unique IDs are different +// For the second event, the service throws when trying to get the same ID twice StatusCode TestUniqueIDGenSvc::execute(const EventContext&) const { - uint evt_num = 4; - uint run_num = 3; + ++m_counter; + uint32_t evt_num = 4; + uint32_t run_num = 3 + m_counter.sum(); std::string name = "Some algorithm name"; auto uid = m_service->getUniqueID(evt_num, run_num, name); - auto uid_again = m_service->getUniqueID(evt_num, run_num, name); - - if (uid != uid_again) { - return StatusCode::FAILURE; + auto uid_again = m_service->getUniqueID(evt_num + (m_counter.sum() % 2), run_num, name); + if (uid == uid_again) { + throw std::runtime_error("Unique IDs are the same"); } return StatusCode::SUCCESS; diff --git a/test/k4FWCoreTest/src/components/TestUniqueIDGenSvc.h b/test/k4FWCoreTest/src/components/TestUniqueIDGenSvc.h index b0bfa656..5658c6b7 100644 --- a/test/k4FWCoreTest/src/components/TestUniqueIDGenSvc.h +++ b/test/k4FWCoreTest/src/components/TestUniqueIDGenSvc.h @@ -20,7 +20,8 @@ #define TEST_UNIQUEIDGENSVC_H // GAUDI -#include +#include "Gaudi/Accumulators.h" +#include "Gaudi/Algorithm.h" #include "k4Interface/IUniqueIDGenSvc.h" @@ -37,7 +38,8 @@ class TestUniqueIDGenSvc : public Gaudi::Algorithm { StatusCode execute(const EventContext&) const final; private: - SmartIF m_service; + SmartIF m_service; + mutable Gaudi::Accumulators::Counter<> m_counter{this, "EventCounter"}; }; #endif // TEST_UNIQUEIDGENSVC_H