Skip to content

AthenaMonitoringKernel: Remove temporary vector creation

Frank Winklmeier requested to merge fwinkl/athena:athenamon_vectorrep into master

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)

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
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.

Edited by Frank Winklmeier

Merge request reports