Use local memory pool in EvtStoreSvc

This adds some custom arena/allocator utilities (inspired by, but different from, lhcb/LHCb!2197 (merged)) and uses them inside EvtStoreSvc to minimise calls to the standard new/delete. Improves LHCb/HLT1 throughput by around 1.5%.

This handles everything internally to the EvtStoreSvc component, so it should be self-contained. Conceptually it would be nicer if the resources were owned externally, by the EventContext, but I didn't do anything like that here.

cc: @nnolte @graven

I like it!

Another related speedup which I really would like to see, but which is a lot more invasive is the use of small buffer optimization for the 'payload' itself -- but as long as the interface is in terms of DataObject* this is impossible, as one needs to know what is being stored. So that can only be done in conjunction with a (templated) interface for 'put', and leaving the type-erasure to the store, i.e. make AnyDataWrapper obsolete / integrate it into the store. Because we have lots of std::vector type data objects, an SSO which would fit say 24 bytes would avoid all those small allocations!). Unfortunately, to implement that requires switching to a different datahandle, and to do that we should first make an experiment specific layer which sets the defaults for the Gaudi::Functional traits, as then we can switch the default handle types with one switch...

And then another speedup would be to just replace std::unordered_map with a better hash map, eg. bytell_hash_map described here

Yeah, after all of my changes then AnyDataWrapper is the main surviving source of dynamic [de]allocation.

If we could completely defer the type-erasure to the store then the fast memory pool implemented here could be used instead of (or as well as) SSO, but I couldn't see how to do that non-invasively...

mentioned in merge request lhcb/LHCb!2270 (merged)

mentioned in merge request lhcb/Rec!1845 (closed)

mentioned in merge request lhcb/Phys!631 (closed)

@graven one other thing I came across is that with the LHCb scheduler the event store is cleared twice per event:

• At the start of the event the scheduler calls setRoot, which internally calls clearStore
• At the end of the event the scheduler calls clearStore followed by freeStore

It would be nice to tweak things so that it is only cleared once per event. Do you have a preferred solution?

(cc: @nnolte)

No real preference on my side -- but from first principles, I think we should call clearStore at the end of an event (even if just to get the 'boundary' condition right on the last event, without having to handle it as a 'special case' ). We also have to call setRoot at the start of the event -- now, perhaps a compromise would be that setRoot has as a precondition that the store is empty, and instead of 'making really sure it empty' it should just check, and assert/throw/... if it isn't. So I'm slightly leaning towards making the scheduler responsible for this, and not having setRoot implicitly clearing -- but then that behaviour should be consistent amongst all the store implementations we use/have...

added 7 commits

• 52a7d33e...2c7e21d5 - 2 commits from branch gaudi:master
• d1a34fe2 - Add Gaudi::Allocator::Arena<Resource, T> generic allocator holding a pointer to a memory resource.
• 6334648c - Add Gaudi::Arena::Monotonic<Alignment, UpstreamAllocator> memory resource and...
• 9c625f1b - Add reset() method to Gaudi::Arena::Monotonic.
• 326940a4 - Migrate EvtStoreSvc to use Gaudi::Allocator::MonotonicArena<T> with an...
• 76757316 - Modify EvtStoreSvc to assume+check that the store is empty in setRoot instead...

Compare with previous version

@graven I tweaked it a bit more. Given that we already had the concept of a fixed set of slots, I took my changes slightly further and now re-use the pool for many events, plus removed the bonus clearStore as discussed before.

It would be nice to get rid of some of the tuning properties used for reserving etc and instead dynamically track the 99.9% percentile of the map capacity or whatever, but I didn't do that here...

added 2 commits

• e36757cb - Migrate EvtStoreSvc to use Gaudi::Allocator::MonotonicArena<T> with an...
• efb734a7 - Modify EvtStoreSvc to assume+check that the store is empty in setRoot instead...

Compare with previous version

added 5 commits

• 5b257253 - Add Gaudi::Allocator::Arena<Resource, T> generic allocator holding a pointer to a memory resource.
• 44c0fa92 - Add Gaudi::Arena::Monotonic<Alignment, UpstreamAllocator> memory resource and...
• df62288d - Add reset() method to Gaudi::Arena::Monotonic.
• 3973a0aa - Migrate EvtStoreSvc to use Gaudi::Allocator::MonotonicArena<T> with an...
• 6109692b - Modify EvtStoreSvc to assume+check that the store is empty in setRoot instead...

Compare with previous version

added 5 commits

• 84648c7e - Add Gaudi::Allocator::Arena<Resource, T> generic allocator holding a pointer to a memory resource.
• 01cb22ac - Add Gaudi::Arena::Monotonic<Alignment, UpstreamAllocator> memory resource and...
• cf5a197d - Migrate EvtStoreSvc to use Gaudi::Allocator::MonotonicArena<T> with an...
• a0f346db - Modify EvtStoreSvc to assume+check that the store is empty in setRoot instead...
• 173eddda - Add unit test for Gaudi::Allocator::Arena and Gaudi::Arena::Monotonic.

Compare with previous version

unmarked as a Work In Progress

changed the description

/ci-test

mentioned in issue #109

mentioned in issue #110

Assuming the /ci-test doesn't throw up any surprises, I think this is good to go. Please let me know if you disagree!

added 2 commits

• 7a2f3262 - Modify EvtStoreSvc to assume+check that the store is empty in setRoot instead...
• 94dd672f - Add unit test for Gaudi::Allocator::Arena and Gaudi::Arena::Monotonic.

Compare with previous version

Pushed a fix for the clang warnings.

added lhcb-test-throughput2 label

mentioned in issue lhcb-core/LbNightlyTools#44

mentioned in merge request lhcb/LHCb!2307 (merged)

mentioned in issue lhcb/Rec#89 (closed)

• [2020-01-14 14:13] Validation started with lhcb-test-throughput2#260
• [2020-01-14 14:29] Validation started with lhcb-test-throughput2#260
• [2020-01-14 16:00] Validation started with lhcb-test-throughput2#260
• [2020-01-14 16:51] Validation started with lhcb-test-throughput2#260
• [2020-01-14 17:16] Validation started with lhcb-test-throughput2#260
• [2020-01-14 17:22] Automatic merge failed in lhcb-test-throughput2#260
• [2020-01-14 17:55] Automatic merge failed in lhcb-test-throughput2#260
• [2020-01-14 18:00] Automatic merge failed in lhcb-test-throughput2#260
• [2020-01-14 18:03] Automatic merge failed in lhcb-test-throughput2#260
• [2020-01-14 18:04] Automatic merge failed in lhcb-test-throughput2#260
• [2020-01-15 11:30] Automatic merge failed in lhcb-test-throughput2#260
• [2020-01-15 13:40] Automatic merge failed in lhcb-test-throughput2#260
• [2020-01-15 13:56] Automatic merge failed in lhcb-test-throughput2#260
• [2020-01-15 14:13] Automatic merge failed in lhcb-test-throughput2#260
• [2020-01-15 14:49] Automatic merge failed in lhcb-test-throughput2#260
• [2020-01-15 14:53] Automatic merge failed in lhcb-test-throughput2#260
• [2020-01-15 14:57] Automatic merge failed in lhcb-test-throughput2#260
• [2020-01-16 16:28] Validation started with lhcb-test-throughput2#262
• [2020-01-16 16:32] Validation started with lhcb-test-throughput2#262
• [2020-01-16 16:35] Validation started with lhcb-test-throughput2#262
• [2020-01-16 16:48] Validation started with lhcb-test-throughput2#262
• [2020-01-16 16:52] Validation started with lhcb-test-throughput2#262
• [2020-01-16 17:06] Validation started with lhcb-test-throughput2#262
• [2020-01-16 17:13] Validation started with lhcb-test-throughput2#262
• [2020-01-16 17:28] Validation started with lhcb-test-throughput2#262
• [2020-01-16 17:39] Validation started with lhcb-test-throughput2#262
• [2020-01-16 17:54] Validation started with lhcb-test-throughput2#262
• [2020-01-16 18:01] Validation started with lhcb-test-throughput2#262
• [2020-01-16 18:07] Validation started with lhcb-test-throughput2#262
• [2020-01-17 10:02] Validation started with lhcb-test-throughput2#262
• [2020-01-17 11:07] Validation started with lhcb-test-throughput2#262
• [2020-01-17 11:13] Validation started with lhcb-test-throughput2#262
• [2020-01-17 11:22] Validation started with lhcb-test-throughput2#262
• [2020-01-17 11:30] Validation started with lhcb-test-throughput2#262
• [2020-01-17 11:34] Validation started with lhcb-test-throughput2#262
• [2020-01-23 09:59] Automatic merge failed in lhcb-test-throughput2#260
• [2020-01-24 09:26] Automatic merge failed in lhcb-test-throughput2#260
• [2020-01-24 09:35] Automatic merge failed in lhcb-test-throughput2#260

added 5 commits

• 226add8c - Add Gaudi::Allocator::Arena<Resource, T> generic allocator holding a pointer to a memory resource.
• 651068f6 - Add Gaudi::Arena::Monotonic<Alignment, UpstreamAllocator> memory resource and...
• 5eef61b1 - Migrate EvtStoreSvc to use Gaudi::Allocator::MonotonicArena<T> with an...
• d462a8cd - Modify EvtStoreSvc to assume+check that the store is empty in setRoot instead...
• 1fd89b6f - Add unit test for Gaudi::Allocator::Arena and Gaudi::Arena::Monotonic.

Compare with previous version

The changes I just pushed are very minor (adding a resource() accessor to the custom allocator, compatible with std::pmr::polymorphic_allocator etc.) and should not have any impact on performance.

removed lhcb-test-throughput2 label

mentioned in issue lhcb/Moore#128 (closed)

assigned to @clemenci

changed milestone to %v33r1

mentioned in merge request lhcb/Moore!377 (merged)

mentioned in issue lhcb/LHCb#73 (closed)

resolved all threads

mentioned in issue #113

added 38 commits

• 1fd89b6f...9a48ebc9 - 33 commits from branch gaudi:master
• 86e5fc67 - Add Gaudi::Allocator::Arena<Resource, T> generic allocator holding a pointer to a memory resource.
• 831b9e4f - Add Gaudi::Arena::Monotonic<Alignment, UpstreamAllocator> memory resource and...
• 7756a5ab - Migrate EvtStoreSvc to use Gaudi::Allocator::MonotonicArena<T> with an...
• a86e51e0 - Modify EvtStoreSvc to assume+check that the store is empty in setRoot instead...
• 32bbddb7 - Add unit test for Gaudi::Allocator::Arena and Gaudi::Arena::Monotonic.

Compare with previous version

Rebased on master to resolve conflicts.

@clemenci as far as I'm concerned this is good to go. Feel free to ping me if there's something you want to be addressed!

added lhcb-gaudi-head lhcb-lcg-dev3 lhcb-lcg-dev4 lhcb-run2-patches-dev4 labels

• [2020-02-11 00:07] Validation started with lhcb-lcg-dev4#1185
• [2020-02-11 00:07] Validation started with lhcb-lcg-dev3#1173
• [2020-02-11 00:08] Validation started with lhcb-gaudi-head#2528
• [2020-02-11 00:10] Validation started with lhcb-run2-patches-dev4#234
• [2020-02-12 00:04] Validation started with lhcb-gaudi-head#2529
• [2020-02-12 00:06] Validation started with lhcb-lcg-dev3#1174
• [2020-02-12 00:07] Validation started with lhcb-lcg-dev4#1186
• [2020-02-12 00:09] Validation started with lhcb-run2-patches-dev4#235
• [2020-02-13 00:03] Validation started with lhcb-lcg-dev4#1187
• [2020-02-13 00:04] Validation started with lhcb-lcg-dev3#1175
• [2020-02-13 00:05] Validation started with lhcb-gaudi-head#2530
• [2020-02-13 00:09] Validation started with lhcb-run2-patches-dev4#236
• [2020-02-14 00:04] Validation started with lhcb-lcg-dev3#1176
• [2020-02-14 00:04] Validation started with lhcb-gaudi-head#2531
• [2020-02-14 00:07] Validation started with lhcb-lcg-dev4#1188
• [2020-02-14 00:09] Validation started with lhcb-run2-patches-dev4#237
• [2020-02-14 13:58] Validation started with lhcb-gaudi-head#2532
• [2020-02-15 00:05] Validation started with lhcb-lcg-dev3#1177
• [2020-02-15 00:06] Validation started with lhcb-lcg-dev4#1189
• [2020-02-15 00:08] Validation started with lhcb-gaudi-head#2533
• [2020-02-15 00:13] Validation started with lhcb-run2-patches-dev4#238
• [2020-02-16 00:03] Validation started with lhcb-lcg-dev3#1178
• [2020-02-16 00:04] Validation started with lhcb-lcg-dev4#1190
• [2020-02-16 00:04] Validation started with lhcb-gaudi-head#2534
• [2020-02-16 00:11] Validation started with lhcb-run2-patches-dev4#239
• [2020-02-17 00:03] Validation started with lhcb-lcg-dev4#1191
• [2020-02-17 00:07] Validation started with lhcb-lcg-dev3#1179
• [2020-02-17 00:07] Validation started with lhcb-gaudi-head#2535
• [2020-02-17 00:14] Validation started with lhcb-run2-patches-dev4#240
• [2020-02-18 00:04] Validation started with lhcb-lcg-dev3#1180
• [2020-02-18 00:04] Validation started with lhcb-lcg-dev4#1192
• [2020-02-18 00:07] Validation started with lhcb-gaudi-head#2536
• [2020-02-18 00:11] Validation started with lhcb-run2-patches-dev4#241
• [2020-02-19 00:04] Validation started with lhcb-lcg-dev4#1193
• [2020-02-19 00:04] Validation started with lhcb-lcg-dev3#1181
• [2020-02-19 00:05] Validation started with lhcb-gaudi-head#2537
• [2020-02-19 00:11] Validation started with lhcb-run2-patches-dev4#242
• [2020-02-20 00:05] Validation started with lhcb-lcg-dev4#1194
• [2020-02-20 00:08] Validation started with lhcb-gaudi-head#2538
• [2020-02-20 00:16] Validation started with lhcb-run2-patches-dev4#243
• [2020-02-21 00:03] Validation started with lhcb-lcg-dev3#1183
• [2020-02-21 00:03] Validation started with lhcb-lcg-dev4#1195
• [2020-02-21 00:08] Validation started with lhcb-run2-patches-dev4#244
• [2020-02-21 11:17] Validation started with lhcb-lcg-dev4#1196
• [2020-02-23 00:04] Validation started with lhcb-gaudi-head#2539
• [2020-02-23 00:04] Validation started with lhcb-lcg-dev3#1184
• [2020-02-23 00:06] Validation started with lhcb-lcg-dev4#1197
• [2020-02-23 00:11] Validation started with lhcb-run2-patches-dev4#245
• [2020-02-24 00:04] Validation started with lhcb-lcg-dev3#1185
• [2020-02-24 00:07] Validation started with lhcb-lcg-dev4#1198
• [2020-02-24 00:10] Validation started with lhcb-run2-patches-dev4#246

approved this merge request

merged

mentioned in commit 7e9cfd89

mentioned in merge request !1187 (closed)