-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Exclude PluginManager cache from MaxMemoryPreload monitoring #46542
base: master
Are you sure you want to change the base?
Exclude PluginManager cache from MaxMemoryPreload monitoring #46542
Conversation
cms-bot internal usage |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-46542/42411 |
A new Pull Request was created by @makortel for master. It involves the following packages:
@Dr15Jones, @cmsbuild, @makortel, @smuzaffar can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
@cmsbuild, please test |
@Dr15Jones please review |
-1 Failed Tests: RelVals-INPUT
RelVals-INPUT
Expand to see more relval errors ...
Comparison SummarySummary:
|
45876c8
to
ab2cab7
Compare
@cmsbuild, please test |
Pull request #46542 was updated. @Dr15Jones, @makortel, @smuzaffar can you please check and sign again. |
@cmsbuild, please abort |
@cmsbuild, please test |
-1 Failed Tests: RelVals-INPUT RelVals-INPUT
Comparison SummarySummary:
|
Comparison differences are related to #46416 |
This pull request is fully signed and it will be integrated in one of the next master IBs (but tests are reportedly failing). This pull request will now be reviewed by the release team before it's merged. @sextonkennedy, @mandrenguyen, @antoniovilela, @rappoccio (and backports should be raised in the release meeting by the corresponding L2) |
+core |
This pull request is fully signed and it will be integrated in one of the next master IBs (but tests are reportedly failing). This pull request will now be reviewed by the release team before it's merged. @antoniovilela, @sextonkennedy, @rappoccio, @mandrenguyen (and backports should be raised in the release meeting by the corresponding L2) |
ignore tests-rejected with external-failure |
PR description:
As described in #46359, developer areas with many checked out packages result in larger "max memory" reported by MaxMemoryPreload AllocMonitor. This additional memory is used by PluginManager cache.
As a simple (albeit a bit hacky) workaround, this PR adds hooks to the MaxMemoryPreload that allow its data collection to be paused, and uses those hooks in PluginManager to paused the data collection during the cache file reading.
Resolves #46359
Resolves cms-sw/framework-team#1074
PR validation:
Tested the impact with two developer areas of CMSSW_14_2_0_pre2: one with many packages checked out, and one with only two packages (
FWCore/PluginManager
andPerfTools/MaxMemoryPreload
). The remaining difference in "max memory" was about 1 MB, or 0.05 %.