AthenaMonitoringKernel: Remove temporary vector creation
Replace the get[String]VectorRepresenation()
functions that return a
std::vector
with a copy of the monitored values by index-based
accessor functions get[String](size_t i)
.
This means we can only support random access containers, i.e.
std::set
, set::list
, std::map
, etc. are no longer supported! Apart
from one unit test, they do not seem to be used. Implementing a proper
iterator would be quite a bit more complicated.
Some measurements:
(best of 10 by running GenericMonPerf_test.exe -r100
with tcmalloc)
- Athena,master,2020-05-17 callgrind.out.23622
fillFromScalar MON: 3931 +- 266 us ROOT: 143 +- 2 us
fillFromMultipleScalars MON: 5610 +- 798 us ROOT: 372 +- 6 us
fillFromCollection MON: 3267 +- 10 us ROOT: 517 +- 1 us
- This MR on top callgrind.out.17898
fillFromScalar MON: 3784 +- 8 us ROOT: 165 +- 1 us
fillFromMultipleScalars MON: 5264 +- 578 us ROOT: 375 +- 13 us
fillFromCollection MON: 3426 +- 26 us ROOT: 521 +- 13 us
Considering the uncertainties the effect it not really measurable. Judging from the callgrind profiles there is a reduction in total new/free calls from 120k to 80k.
I am open to opinions if we should go ahead with this change nevertheless (are there any downsides?) or drop it.
TODO:
-
Check histograms are identical with HLT and DQ job -
Measure actual performance gains on synthetic benchmark and above jobs -
Adapt the unit testing (i.e. the code in test/mocks/
no longer compiles)
cc @tbold @stelzer @ponyisi @cburton
Relates to ATR-21210.