Repository navigation
[hist] Optimize bin label search - #23682
Conversation
Test Results 24 files 24 suites 4d 5h 15m 29s ⏱️ For more details on these failures, see this check. Results for commit ee6847f. ♻️ This comment has been updated with latest results. |
caebd84 to
dbb2422
Compare
|
This might help in the majority of cases (to be validated), but a way to be sure to fix the original issue is to pre-sort all labels once before the loop, to change the complexity from O(nbins^2) to O(nbins). std::vector<const TObjString *> sortedLabels(labelsX->GetEntries() + labelsY->GetEntries() + labelsZ->GetEntries());
for (const auto *label : TRangeStaticCast<const TObjString>(labelsX))
sortedLabels[label->GetUniqueId()] = label;
for (const auto *label : TRangeStaticCast<const TObjString>(labelsY))
sortedLabels[labelsX->GetEntries() + label->GetUniqueId()] = label;
// ...
// now we can call sortedLabels[binx] instead of hist->GetXaxis()->GetBinLabel(binx) in the loop |
dbb2422 to
8251189
Compare
silverweed
left a comment
silverweed
left a comment
There was a problem hiding this comment.
We're not really avoiding linear scaling (that's pretty much unavoidable given the situation), so the commit message should probably not mention it
| EXPECT_DOUBLE_EQ(hist.Interpolate(0.3, 19.9, 5.), 100. * 19 + 10000. * 4.5); | ||
| EXPECT_DOUBLE_EQ(hist.Interpolate(0.8, 19.9, 5.), 0.3 + 100. * 19 + 10000. * 4.5); | ||
| } | ||
|
|
There was a problem hiding this comment.
| #include "TList.h" | |
There was a problem hiding this comment.
thanks for the comment, and to debian13 that failed to compile!
and add a unit test.
Before linear search, assume the bin labels are ordered by unique id. Speeds up merging, reported to be slow by root-project#23681
8251189 to
25978bc
Compare
25978bc to
ee6847f
Compare
Good point, thanks. Done. |
silverweed
left a comment
silverweed
left a comment
There was a problem hiding this comment.
lgtm after addressing the last comment
therefore speeding up identification of the bin label. Fixes root-project#23681
ee6847f to
bad8033
Compare
|
/backport to 6.40 |
|
Preparing to backport PR #23682 to branch 6.40 requested by dpiparo |
|
This PR has been backported to branch 6.40: #23737 |
4 participants
Before linear search, assume the bin labels are ordered by unique id.
Fixes #23681
En passant, also fixes #23716