Igprof analysis of 11_1_0_p3_ROOT618 : CSCMotherboard::CSCMotherboard

[tommasoboccali]

Dear all,
an igprof analysis of L1+RECO here shows a large allocation (~ 220 MB) in

CSCMotherboard::CSCMotherboard(unsigned int, unsigned int, unsigned int, unsigned int, unsigned int, edm::ParameterSet const&)

direct link to the report HERE

Can you please have a look whether this is expected / correct?
cheers

[cmsbuild]

A new Issue was created by @tommasoboccali Tommaso Boccali.

@Dr15Jones, @silviodonato, @dpiparo, @smuzaffar, @makortel can you please review it and eventually sign/assign? Thanks.

cms-bot commands are listed here

[makortel]

assign l1

[cmsbuild]

New categories assigned: l1

@benkrikler,@rekovic you have been requested to review this Pull request/Issue and eventually sign? Thanks

[makortel]

It looks tome that the main culprit is

cmssw/L1Trigger/CSCTriggerPrimitives/interface/CSCCathodeLCTProcessor.h

Line 201 in 0393b55

PulseArray hitsCLCT[99];

coming via

cmssw/L1Trigger/CSCTriggerPrimitives/src/CSCMotherboard.cc

Line 50 in 0393b55

clctProc.reset(new CSCCathodeLCTProcessor(endcap, station, sector, subsector, chamber, conf));

The PulseArray is defined as

cmssw/L1Trigger/CSCTriggerPrimitives/interface/CSCCathodeLCTProcessor.h

Line 101 in 0393b55

typedef unsigned int PulseArray[CSCConstants::NUM_LAYERS][CSCConstants::NUM_HALF_STRIPS_7CFEBS];

and NUM_LAYERS is 6 and NUM_HALF_STRIPS_7CFEBS is 225. That makes 6*225*99*4 bytes = 523 kB. GDB gave me 540 hits in the CSCMotherboard constructor (in the workflow mentioned in #30742 (comment)), which makes the total 540*523 kB = 275 MB (this doesn't exactly match to the IgProf report though).

FYI @dildick

[dildick]

I think that the member PulseArray hitsCLCT[99]; was removed in a recent CMSSW_11_2_X version. See #30103. I need to keep track of all the hits for each CLCT candidate for each BX. See https://github.com/cms-sw/cmssw/blob/master/L1Trigger/CSCTriggerPrimitives/src/CSCCathodeLCTProcessor.cc#L573-L575. This container is filled with hits for each CLCT candidate https://github.com/cms-sw/cmssw/blob/master/L1Trigger/CSCTriggerPrimitives/src/CSCCathodeLCTProcessor.cc#L892

[tommasoboccali]

Dear Sven, I am not sure I get this comment. Is this for the memory reduction, or a real bug? If the latter, does it *need* to be ported also in 11_1_X? (by the way, I cannot even see the comment here in github, I only got it from a github email... strange)

…

On Tue, Jul 28, 2020 at 6:51 PM Sven Dildick ***@***.***> wrote: This condition https://github.com/cms-sw/cmssw/blob/master/L1Trigger/CSCTriggerPrimitives/src/CSCTriggerPrimitivesBuilder.cc#L340 should be else if (isSLHC_ and ring == 1 and ((stat == 2 and runME21Up_ and !runME21ILT_) || (stat == 3 and runME31Up_) || (stat == 4 and runME41Up_))) { — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#30745 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AA7HMYOGZY4ML6HGHJRKK7DR536WTANCNFSM4O3ZF2JA> .

-- Tommaso Boccali INFN Pisa

[dildick]

The comment posted earlier is only relevant for a different issue.

After removing PulseArray hitsCLCT[99];, is the large allocation still there?

[dildick]

@tommasoboccali Should have a look at newer versions of the CSC trigger if the situation has changed?

[tommasoboccali]

Ciao!
the HLT production has started, so we are not in the critical path for that.
Still, in the long run we will need to reduce the memory footprint of Phase-2 sw, so yes, a look would be appreciated 👍

thanks

tom