Follow-up from "Add velo lumi counters to HLT2"
This comment !3757 (comment 7645246) and The following discussion from !3757 (merged) should be addressed:
-
@graven started a discussion: (+4 comments) Let's avoid unnecessary string operations (which are not cheap, as they involve heap memory allocations when the strings become more than O(20) characters long):
// create a map of counter names to hits vector std::map<std::string, double> counterMap; // assign as keys the counter names and init the values to 0 for ( auto i = 0u; i < m_counterNames.size(); ++i ) { counterMap[ m_counterNames[i]] = 0; } counterMap["Vertices"] = vertices.size(); auto vertexIndex{0u}; for ( auto vertex : vertices ) { auto pos = vertex.position(); auto rho = std::sqrt( pos.x() * pos.x() + pos.y() * pos.y() ); auto absz = std::abs( pos.z() ); if ( absz < 300.f * Gaudi::Units::mm && rho < 3.f * Gaudi::Units::mm ) { ++counterMap["FiducialVertices"]; } // use the sum of vertex Z co-ordinates modulo the number of vertices to pseudo-randomly choose a vertex to record vertexIndex += static_cast<int>( absz ); } if ( vertices.size() > 0u ) { auto vtx = vertices[vertexIndex % vertices.size()].position(); counterMap["VertexX"] = vtx.x(); counterMap["VertexY"] = vtx.y(); counterMap["VertexZ"] = vtx.z(); } LHCb::HltLumiSummary summary{}; for ( auto i = 0u; i < m_counterNames.size(); ++i ) { summary.addInfo( m_counterBaseName + m_counterNames[i], counterMap[m_counterNames[i]] ); }
Now, when combined with the change below where the
m_counterNames
is instead a vector of enums, it becomes possible to avoid the string operations all together:// create a map of counter names to hits vector std::map<CounterNames, double> counterMap; // assign as keys the counter names and init the values to 0 for ( auto i = 0u; i < m_counterNames.size(); ++i ) { counterMap[ m_counterNames[i]] = 0; } counterMap[CounterNames::Vertices] = vertices.size(); auto vertexIndex{0u}; for ( auto vertex : vertices ) { auto pos = vertex.position(); auto rho = std::sqrt( pos.x() * pos.x() + pos.y() * pos.y() ); auto absz = std::abs( pos.z() ); if ( absz < 300.f * Gaudi::Units::mm && rho < 3.f * Gaudi::Units::mm ) { ++counterMap[CounterNames::FiducialVertices]; } // use the sum of vertex Z co-ordinates modulo the number of vertices to pseudo-randomly choose a vertex to record vertexIndex += static_cast<int>( absz ); } if ( vertices.size() > 0u ) { auto vtx = vertices[vertexIndex % vertices.size()].position(); counterMap[CounterNames::VertexX] = vtx.x(); counterMap[CounterNames::VertexY] = vtx.y(); counterMap[CounterNames::VertexZ] = vtx.z(); } LHCb::HltLumiSummary summary{}; for ( auto i = 0u; i < m_counterNames.size(); ++i ) { summary.addInfo( m_counterBaseName + toString(m_counterNames[i]), counterMap[m_counterNames[i]] ); }
Given that at this point, the map has as key a small, dense set of small integer values, the
std::map
is overkill, but replacing that is left as an exercise to the author ;-)