Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[14_0_X] Remove redundant data members from InputFileCatalog to reduce memory use #47063

Open
wants to merge 4 commits into
base: CMSSW_14_0_X
Choose a base branch
from
Open

[14_0_X] Remove redundant data members from InputFileCatalog to reduce memory use #47063

wants to merge 4 commits into from

Conversation

makortel
Copy link
Contributor

@makortel makortel commented Jan 8, 2025

PR description:

Backport of #47013

PR validation:

This branch is the original development. See description of #47013 for more details

If this PR is a backport please specify the original PR and why you need to backport that PR. If this PR will be backported please specify to which release cycle the backport is meant for:

Backport of #47013

@makortel
Add tests for InputFileCatalog
4a3e493
@makortel
Remove InputFileCatalog::fileNames_ as seemingly unnecessary
a18ff42 
The fileNames_ was used only in init(), and before being modified in
init() it was a direct copy of logicalFileNames_. There doesn't seem
to be any real need to store it as a data member, and removing it
allows to save ~100 MB memory per stream on an example MC production
(using premixing) DIGI job whose configuration specified had ~500k
pileup files.
@makortel
Remove InputFileCatalog::logicalFileNames as unnecessary
3d5c752 
The logicalFileNames() was not used, so removing the member allows
avoiding one copy of the LFNs.
@makortel
Take fileNames by value in InputFileCatalog constructor
e9e9746 
The constructor makes a copy of the fileNames anyway. For the cases
where InputFileCatalog is constructed with temporary or moved
vector<string>, this change allows to avoid copying that vector.

The Sources tend to pass in directly a temporary vector<string> from
the ParameterSet, so there could be a visible reduction in memory
churn.
@makortel makortel mentioned this pull request Jan 8, 2025
Open
3 tasks
@cmsbuild cmsbuild added this to the CMSSW_14_0_X milestone Jan 8, 2025
@cmsbuild
Copy link
Contributor

cmsbuild commented Jan 8, 2025

A new Pull Request was created by @makortel for CMSSW_14_0_X.

It involves the following packages:

  • FWCore/Catalog (core)
  • FWCore/Sources (core)

@Dr15Jones, @cmsbuild, @makortel, @smuzaffar can you please review it and eventually sign? Thanks.
@missirol, @wddgit this is something you requested to watch as well.
@antoniovilela, @mandrenguyen, @rappoccio, @sextonkennedy you are the release manager for this.

cms-bot commands are listed here

@cmsbuild
Copy link
Contributor

cmsbuild commented Jan 8, 2025 •
edited
Loading

cms-bot internal usage

@makortel
Copy link
Contributor Author

makortel commented Jan 8, 2025

@cmsbuild, please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Jan 8, 2025

+1

Size: This PR adds an extra 16KB to repository
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-3c405b/43682/summary.html
COMMIT: e9e9746
CMSSW: CMSSW_14_0_X_2025-01-08-1100/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/47063/43682/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

@makortel makortel mentioned this pull request Jan 9, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
core-pending orp-pending pending-signatures tests-approved
Projects
None yet
Milestone
CMSSW_14_0_X
Development

Successfully merging this pull request may close these issues.
2 participants
@makortel @cmsbuild