Algorithm state profiling

Added a method to the scheduler to return the number of algorithms in each state.

Useful for profiling multithreaded jobs where we see inefficient use of threads: are we limited by our CF/DF graph, by algorithm cardinality, or by some other locking mechanism?

changed milestone to %v33r2

added task scheduling label

added 1 commit

• f11f41c6 - Fixed formatting

Compare with previous version

added 1 commit

• f004ef64 - Move the vector of all states to a private member, and make it const

Compare with previous version

added 1 commit

• f6e22eb6 - Fixed formatting

Compare with previous version

Tagging @ishapova and @leggett - this is the level of information that we think would be useful for the HLT, but would some other structure be better for a wider context?

We could potentially just dump the entire alg/state map, but that seemed excessive.

Expected usage is for our monitoring system to poll this method at a fixed sampling interval, rather than keyed to a particular event (although obviously that could be done too).

Regards, Ben

In general, I support having this sort of functionality. It may indeed be useful in several situations. But I have the following concerns about this addition:

1. I don't think it is thread-safe. Underlying state sets can be changed by actions running in the control thread while you're counting. This will result in both misses (false underoccupancy) and duplicates (false oversubscription) in state totals.
2. Perhaps, it would be semantically more appropriate to make this part of AlgExecStateSvc? The latter might already have some relevant methods, though - as is - with narrower state sets. In this case, however, a client of the interface would need to have access to all active event contexts. IIRC, we don't track that on the Gaudi side (apart from the scheduler where we can access those indirectly). Were you intending to access the interface from the HLT event loop?

added 1 commit

• a4b85a4d - Count states in the updateStates method, for thread safety

Compare with previous version

added 1 commit

• f4296611 - Fixed formatting

Compare with previous version

added 1 commit

• 6486d856 - Changed method name to occupancy. Added mutex

Compare with previous version

added 42 commits

• 6486d856...c7d6fdfd - 41 commits from branch gaudi:master
• 64e4f669 - Merge remote-tracking branch 'upstream/master' into AlgStateProfiling

Compare with previous version

added 1 commit

• a047de4d - Fixed formatting

Compare with previous version

assigned to @clemenci

resolved all threads

marked as a Work In Progress

changed the description

Having had another HLT meeting, I think we have a different proposal for the data to collect, which should be more thread-safe. WIP for now.

added 2 commits

• 46bde1f8 - Changed activation to be within the event loop, rather than at config time
• 1a202f5c - Allow profiling enable/disable from inside event loop. Locking using concurrent_vector

Compare with previous version

unmarked as a Work In Progress

resolved all threads

added 2 commits

• 5dafd4ce - Fixed formatting
• 688526e8 - Surplus include

Compare with previous version

added 1 commit

• 5c806809 - Missed initialisation

Compare with previous version

added 1 commit

• a7eb5af5 - Changed to an internal timer and queue of snapshots

Compare with previous version

added 1 commit

• 2e110a33 - Fixed formatting

Compare with previous version

added 1 commit

• 19e45ed0 - Redundant include

Compare with previous version

added 1 commit

• 2ff895f8 - Greater-than-or-equal-to interval

Compare with previous version

removed milestone

added 2 commits

• d0000032 - Changed to a loop over state values, rather than over a vector containing all states.
• 84428f82 - Merge branch 'AlgStateProfiling' of https://gitlab.cern.ch/bwynne/Gaudi_gaudi...

Compare with previous version

added 1 commit

• 5fcc4728 - Std move

Compare with previous version

added 1 commit

• 7769461d - Fixed formatting

Compare with previous version

added 76 commits

• 7769461d...d1dc2b0e - 73 commits from branch gaudi:master
• de1a27df - Merge remote-tracking branch 'upstream/master' into AlgStateProfiling
• 09e98e73 - Added bounded queue, retrieve data with callback. But about to remove queue completely
• 87a58896 - Remove the queue, process snapshots directly

Compare with previous version

added 1 commit

• 27308d58 - Fixed formatting

Compare with previous version

added 1 commit

• 4f036da7 - std::move the lambda

Compare with previous version

added 1 commit

• add77c63 - Fixed formatting

Compare with previous version

approved this merge request

resolved all threads

I forget who actually has write access to Gaudi, but tagging @leggett and @fwinkl.

@clemenci approved the MR before he went on holiday, so assuming you're OK with it can we get this merged?

added 1 commit

• a5c0d292 - Unnecessary include

Compare with previous version

/ci-test --merge

• [2020-07-29 15:26] Validation started with lhcb-master-mr#1133

resolved all threads

approved this merge request

merged

mentioned in commit 19d53385