Skip to content

Commit 9467ebb

Browse files
committed
DPL Analysis: out-of-line HistogramSpec related construction
The price we pay in compile time is not worth the speedup in registering the histograms, so we move the related methods out of line.
1 parent d0fec95 commit 9467ebb

2 files changed

Lines changed: 57 additions & 39 deletions

File tree

‎Framework/Core/include/Framework/HistogramSpec.h‎

Lines changed: 14 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -84,41 +84,20 @@ using HistPtr = std::variant<std::shared_ptr<THn>, std::shared_ptr<THnSparse>, s
8484
constexpr double VARIABLE_WIDTH = 0.;
8585

8686
struct AxisSpec {
87-
AxisSpec(std::vector<double> binEdges_, std::optional<std::string> title_ = std::nullopt, std::optional<std::string> name_ = std::nullopt)
88-
: nBins(std::nullopt),
89-
binEdges(std::move(binEdges_)),
90-
title(std::move(title_)),
91-
name(std::move(name_))
92-
{
93-
}
94-
95-
AxisSpec(int nBins_, double binMin_, double binMax_, std::optional<std::string> title_ = std::nullopt, std::optional<std::string> name_ = std::nullopt)
96-
: nBins(nBins_),
97-
binEdges({binMin_, binMax_}),
98-
title(std::move(title_)),
99-
name(std::move(name_))
100-
{
101-
if (binMin_ > binMax_) {
102-
LOG(fatal) << "Defined ill-defined axis";
103-
}
104-
}
87+
// Defined out of line in HistogramSpec.cxx on purpose. These run once per
88+
// histogram at init() time, so inlining them buys nothing at runtime, while
89+
// a task that books a few hundred histograms in one init() pays for every
90+
// expansion: each one materialises a std::vector<double> and two
91+
// std::optional<std::string> at the call site, with the matching destructor
92+
// and exception-unwind paths. Under -g that also means a debug record per
93+
// temporary, which is what makes the register allocator's rewrite pass go
94+
// superlinear on such functions.
95+
AxisSpec(std::vector<double> binEdges_, std::optional<std::string> title_ = std::nullopt, std::optional<std::string> name_ = std::nullopt);
96+
97+
AxisSpec(int nBins_, double binMin_, double binMax_, std::optional<std::string> title_ = std::nullopt, std::optional<std::string> name_ = std::nullopt);
10598

10699
// first entry is assumed to be the number of bins; in case of variable size binning it must be set to zero
107-
AxisSpec(ConfigurableAxis binEdges_, std::optional<std::string> title_ = std::nullopt, std::optional<std::string> name_ = std::nullopt)
108-
: nBins(std::nullopt),
109-
binEdges(std::vector<double>(binEdges_)),
110-
title(std::move(title_)),
111-
name(std::move(name_))
112-
{
113-
if (binEdges.empty()) {
114-
return;
115-
}
116-
if (binEdges[0] != VARIABLE_WIDTH) {
117-
nBins = static_cast<int>(binEdges[0]);
118-
binEdges.resize(3); // nBins, lowerBound, upperBound, disregard whatever else is stored in vecotr
119-
}
120-
binEdges.erase(binEdges.begin()); // remove first entry that we assume to be number of bins
121-
}
100+
AxisSpec(ConfigurableAxis binEdges_, std::optional<std::string> title_ = std::nullopt, std::optional<std::string> name_ = std::nullopt);
122101

123102
[[nodiscard]] long getNbins() const;
124103

@@ -138,12 +117,8 @@ struct AxisSpec {
138117
*/
139118
//**************************************************************************************************
140119
struct HistogramConfigSpec {
141-
HistogramConfigSpec(HistType type_, std::vector<AxisSpec> axes_, uint8_t nSteps_ = 1)
142-
: type(type_),
143-
axes(std::move(axes_)),
144-
nSteps(nSteps_)
145-
{
146-
}
120+
// Out of line for the same reason as the AxisSpec constructors above.
121+
HistogramConfigSpec(HistType type_, std::vector<AxisSpec> axes_, uint8_t nSteps_ = 1);
147122
HistogramConfigSpec() = default;
148123
HistogramConfigSpec(HistogramConfigSpec const& other) = default;
149124
HistogramConfigSpec(HistogramConfigSpec&& other) = default;

‎Framework/Core/src/HistogramSpec.cxx‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,49 @@
1515
namespace o2::framework
1616
{
1717

18+
// Out of line: see the comment on the declarations in HistogramSpec.h.
19+
AxisSpec::AxisSpec(std::vector<double> binEdges_, std::optional<std::string> title_, std::optional<std::string> name_)
20+
: nBins(std::nullopt),
21+
binEdges(std::move(binEdges_)),
22+
title(std::move(title_)),
23+
name(std::move(name_))
24+
{
25+
}
26+
27+
AxisSpec::AxisSpec(int nBins_, double binMin_, double binMax_, std::optional<std::string> title_, std::optional<std::string> name_)
28+
: nBins(nBins_),
29+
binEdges({binMin_, binMax_}),
30+
title(std::move(title_)),
31+
name(std::move(name_))
32+
{
33+
if (binMin_ > binMax_) {
34+
LOG(fatal) << "Defined ill-defined axis";
35+
}
36+
}
37+
38+
AxisSpec::AxisSpec(ConfigurableAxis binEdges_, std::optional<std::string> title_, std::optional<std::string> name_)
39+
: nBins(std::nullopt),
40+
binEdges(std::vector<double>(binEdges_)),
41+
title(std::move(title_)),
42+
name(std::move(name_))
43+
{
44+
if (binEdges.empty()) {
45+
return;
46+
}
47+
if (binEdges[0] != VARIABLE_WIDTH) {
48+
nBins = static_cast<int>(binEdges[0]);
49+
binEdges.resize(3); // nBins, lowerBound, upperBound, disregard whatever else is stored in vecotr
50+
}
51+
binEdges.erase(binEdges.begin()); // remove first entry that we assume to be number of bins
52+
}
53+
54+
HistogramConfigSpec::HistogramConfigSpec(HistType type_, std::vector<AxisSpec> axes_, uint8_t nSteps_)
55+
: type(type_),
56+
axes(std::move(axes_)),
57+
nSteps(nSteps_)
58+
{
59+
}
60+
1861
void AxisSpec::makeLogarithmic()
1962
{
2063
if (binEdges.size() > 2) {

0 commit comments

Comments
 (0)