Memory problem in ReReco-Run2024C-JetMET1 pilot

[makortel]

A pilot job of the Run2024C ReReco was killed because of using too much memory
https://cms-unified.web.cern.ch/cms-unified/joblogs/pdmvserv_Run2024C_JetMET1_pilot_241122_102431_7689/50660/DataProcessing/
The job was ran in CMSSW_14_0_19.

This issue is about studying the memory behavior of the job above.

This problem is reported also in https://its.cern.ch/jira/browse/CMSCOMPPR-56784 and https://gitlab.cern.ch/groups/cms-ppd/-/epics/12.

[makortel]

assign core

[cmsbuild]

New categories assigned: core

@Dr15Jones,@makortel,@smuzaffar you have been requested to review this Pull request/Issue and eventually sign? Thanks

[cmsbuild]

cms-bot internal usage

[cmsbuild]

A new Issue was created by @makortel.

@Dr15Jones, @antoniovilela, @makortel, @mandrenguyen, @rappoccio, @sextonkennedy, @smuzaffar can you please review it and eventually sign/assign? Thanks.

cms-bot commands are listed here

[makortel]

Plot from the SimpleMemoryCheck printouts

[Dr15Jones]

Here is measurements using the PeriodicAllocMonitor as well a sampling the RSS during the same job

[makortel]

Some overall properties of the job

• 2638 modules are constructed, of which 469 are destructed soon after because nothing in the job consumes their data products, leaving 2169 modules in total
• 12 instances of PoolOutputModule + 1 instance of DQMRootOutputModule
  • AOD, MiniAOD, NanoEDMAOD and 9 skims

[davidlange6]

perhaps its wiser to use 8 cores like the tier0 does? That would gain quite a lot of memory headroom per core

[makortel]

perhaps its wiser to use 8 cores like the tier0 does?

The failed job was configured to use 8 threads.

[davidlange6]

whoops - i was fooled by some of the plots - sorry for the noise.

[makortel]

First observation with @Dr15Jones was a rediscovery of #46526 (comment) that was fixed in #46543 (14_2_X) / #46567 (14_1_X). The fix is being backported to 14_0_X as part of #46903.

[srimanob]

Please ignore the x-axis at the moment, I just plot from order that RSS info is printed out. The blue plot (data.txt) was done in total 15000 events, while the green (data_fix) is still on going (~6700th event in the plot).

I just made a quick plot from printout of SimpleMemoryCheck, this PR (#46903) should help for memory reduction, but not the leak.

[Dr15Jones]

So I applied the backport to my build area and re-ran the PeriodicAllocMonitor job. Unfortunately after nearly 6000 events processed overnight the machine I was running on was being rebooted for scheduled maintenance. So the comparison isn't complete (note I ran the job single threaded in order to avoid exceeding the VSize limit on the machine).

The backport does show much less outstanding allocation size than the original code. There might still be some upward trend even after the backport but it is hard to tell with the sample I was able to make.

[srimanob]

So I applied the backport to my build area and re-ran the PeriodicAllocMonitor job. Unfortunately after nearly 6000 events processed overnight the machine I was running on was being rebooted for scheduled maintenance. So the comparison isn't complete (note I ran the job single threaded in order to avoid exceeding the VSize limit on the machine).

The backport does show much less outstanding allocation size than the original code. There might still be some upward trend even after the backport but it is hard to tell with the sample I was able to make.

Hi @Dr15Jones

Thx.

Which backport are you testing? The PR I made or other PRs which @makortel proposed to backport also.

[Dr15Jones]

Which backport are you testing? The PR I made or other PRs which @makortel proposed to backport also.

The one you (@srimanob) made.

[srimanob]

Thanks @makortel for the script to make a plot.

I try to run with backport PR, it shows that PR helps to reduce the memory.

Before fix:

With PR to fix the memory:

[Dr15Jones]

I was able to uncover a ~ 1k/event memory leak here

cmssw/EventFilter/L1TRawToDigi/plugins/implementations_stage2/RegionalMuonGMTUnpacker.cc

Lines 47 to 48 in 9a4b9e4

	regionalMuonShowerCollection =
	new RegionalMuonShowerBxCollection(); // To avoid warning re uninitialised collection

This was found using the prototype ModuleEventAllocMonitor

[Dr15Jones]

#46918 fixes the problem in master.

[Dr15Jones]

@srimanob would you like #46918 back ported to CMSSW_14_0 ?

[Dr15Jones]

So after applying the backport #46903 and memory leak fix #46918 (the latter having a much smaller effect) I see that the allocations (using the AllocMonitor system to record new/delete calls) shows much more stable behavior

and comparing the final results for RSS and allocations gives

Here I'm must processing the first file in the job and I'm reading that file locally.

[srimanob]

Hi @Dr15Jones
Thanks very much. As we still need to converge on the release, I would propose to backport #46918

[srimanob]

Here is another backport I made, #46942. It includes

1. [14.1.X] use ReadPrescalesFromFile=False in the GenericTriggerEventFlag of a bunch of DQM modules #46591 (use ReadPrescalesFromFile=False in the GenericTriggerEventFlag of a bunch of DQM modules)
2. Rework Muon candidate selection in few tracker ALCARECO  #46574 (Rework Muon candidate selection in few tracker ALCARECO)
3. Add GEN-SIM information to resonant di-muon TkAl ALCARECO producers #45357 (Add GEN-SIM information to resonant di-muon TkAl ALCARECO producers)

[srimanob]

Note to @antoniovilela @mandrenguyen We need to cut the release 14_0 with memory fix PR early next week. Currently we have

1. [14_0_X] Backports of 46543 and 46451 to fix memory leak #46903
2. [14_0_X] Backports of 46591,45357, and 46574 to fix memory leak (add update ALCARECO) #46942
3. Backport of Fixed memory leak in RegionalMuonGMTUnpacker #46918

FYI @vlimant @malbouis

[Dr15Jones]

The relevant backport is #46950

[srimanob]

Update the list:

1. [14_0_X] Backports of 46543 and 46451 to fix memory leak #46903
2. Fixed memory leak in RegionalMuonGMTUnpacker [14_0] #46950
3. [14_0_X] Another backports to fix memory leak #46954

[srimanob]

Here are plot from my last test:

Without any fix:

With fix of #46903

With fix of #46903 and #46942 (currently replace by #46954)

I seems we can be in good shape together with #46950.