Fix memory leaks in PortableHostCollection deserialization #40492

makortel · 2023-01-12T00:26:25Z

PR description:

Before ROOT allocated the member arrays of layout_ of the newObj, that are then overwritten by ROOTReadStreamer() without deallocation. Setting layout_ as the target as well tells ROOT to not to do those allocations. More details in #40491.

The second commit by @ericcano introduces Layout::ROOTStreamerCleaner() to delete the Layout's member arrays allocated by ROOT (more details in #40491 (comment)).

Resolves #40491

PR validation:

Checked with valgrind that the amount of memory leaks in HeterogeneousCore/AlpakaTest/test/reader.py decreased.

cmsbuild · 2023-01-12T00:33:06Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-40492/33668

This PR adds an extra 12KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File DataFormats/PortableTestObjects/src/alpaka/classes_serial_def.xml modified in PR(s): Add support to multi layout portable collections (any number) #40285

cmsbuild · 2023-01-12T00:33:26Z

A new Pull Request was created by @makortel (Matti Kortelainen) for master.

It involves the following packages:

DataFormats/PortableTestObjects (heterogeneous)

@cmsbuild, @makortel, @fwyzard can you please review it and eventually sign? Thanks.
@missirol, @rovere this is something you requested to watch as well.
@perrotta, @dpiparo, @rappoccio you are the release manager for this.

cms-bot commands are listed here

makortel · 2023-01-12T00:35:32Z

enable gpu

makortel · 2023-01-12T00:35:39Z

@cmsbuild, please test

cmsbuild · 2023-01-12T02:56:23Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-0b8d8c/29931/summary.html
COMMIT: 44d861b
CMSSW: CMSSW_13_0_X_2023-01-11-1100/el8_amd64_gcc11
Additional Tests: GPU
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/40492/29931/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

There are some workflows for which there are errors in the baseline:
140.01 step 3
140.03 step 3
140.104 step 3
140.105 step 3
140.106 step 3
140.107 step 3
140.109 step 3
140.111 step 3
140.112 step 3
140.113 step 3
140.114 step 3
140.115 step 3
140.116 step 3
140.11 step 3
The results for the comparisons for these workflows could be incomplete
This means most likely that the IB is having errors in the relvals.The error does NOT come from this pull request

Summary:

No significant changes to the logs found
Reco comparison results: 5 differences found in the comparisons
DQMHistoTests: Total files compared: 49
DQMHistoTests: Total histograms compared: 3555538
DQMHistoTests: Total failures: 3
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3555513
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 48 files compared)
Checked 211 log files, 162 edm output root files, 49 DQM output files
TriggerResults: no differences found

GPU Comparison Summary

Summary:

No significant changes to the logs found
Reco comparison results: 9 differences found in the comparisons
DQMHistoTests: Total files compared: 4
DQMHistoTests: Total histograms compared: 19862
DQMHistoTests: Total failures: 198
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 19664
DQMHistoTests: Total skipped: 0
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 3 files compared)
Checked 12 log files, 9 edm output root files, 4 DQM output files
TriggerResults: no differences found

Before ROOT allocated the member arrays of layout_ of the newObj, that are then overwritten by ROOTReadStreamer without deallocation. Setting layout_ as the target as well tells ROOT to not to do those allocations. Tested with DataFormats/PortableTestObjects, but should apply to CUDADataFormats/PortableTestObjects as well.

makortel · 2023-01-12T14:46:25Z

Now applying the same fix to CUDADataFormats/PortableTestObjects/src/classes_def.xml as well.

cmsbuild · 2023-01-12T14:56:12Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-40492/33678

This PR adds an extra 12KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File DataFormats/PortableTestObjects/src/alpaka/classes_serial_def.xml modified in PR(s): Add support to multi layout portable collections (any number) #40285

cmsbuild · 2023-01-12T14:56:41Z

Pull request #40492 was updated. @cmsbuild, @makortel, @fwyzard can you please check and sign again.

makortel · 2023-01-12T14:57:23Z

@cmsbuild, please test

cmsbuild · 2023-02-03T19:39:05Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-0b8d8c/30389/summary.html
COMMIT: 0eced58
CMSSW: CMSSW_13_0_X_2023-02-03-1100/el8_amd64_gcc11
Additional Tests: GPU
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/40492/30389/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

You potentially removed 10 lines from the logs
Reco comparison results: 4 differences found in the comparisons
DQMHistoTests: Total files compared: 49
DQMHistoTests: Total histograms compared: 3555495
DQMHistoTests: Total failures: 0
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3555473
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 48 files compared)
Checked 213 log files, 164 edm output root files, 49 DQM output files
TriggerResults: no differences found

GPU Comparison Summary

Summary:

No significant changes to the logs found
Reco comparison results: 0 differences found in the comparisons
DQMHistoTests: Total files compared: 4
DQMHistoTests: Total histograms compared: 19862
DQMHistoTests: Total failures: 67
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 19795
DQMHistoTests: Total skipped: 0
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 3 files compared)
Checked 12 log files, 9 edm output root files, 4 DQM output files
TriggerResults: found differences in 2 / 3 workflows

fwyzard · 2023-02-04T16:22:39Z

@fwyzard Do you have any further concerns, or could we proceed with this PR? I believe it would be useful to have the fixes in pre4.

I'll have another look on Monday.

fwyzard · 2023-02-08T10:33:35Z

please test

fwyzard · 2023-02-08T10:33:41Z

+heterogeneous

cmsbuild · 2023-02-08T10:34:04Z

This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). This pull request will now be reviewed by the release team before it's merged. @perrotta, @dpiparo, @rappoccio (and backports should be raised in the release meeting by the corresponding L2)

cmsbuild · 2023-02-08T13:11:14Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-0b8d8c/30500/summary.html
COMMIT: 0eced58
CMSSW: CMSSW_13_0_X_2023-02-07-2300/el8_amd64_gcc11
Additional Tests: GPU
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/40492/30500/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

You potentially added 12 lines to the logs
Reco comparison results: 4 differences found in the comparisons
DQMHistoTests: Total files compared: 49
DQMHistoTests: Total histograms compared: 3555852
DQMHistoTests: Total failures: 3
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3555827
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 48 files compared)
Checked 213 log files, 164 edm output root files, 49 DQM output files
TriggerResults: no differences found

GPU Comparison Summary

Summary:

No significant changes to the logs found
Reco comparison results: 0 differences found in the comparisons
DQMHistoTests: Total files compared: 4
DQMHistoTests: Total histograms compared: 19862
DQMHistoTests: Total failures: 69
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 19793
DQMHistoTests: Total skipped: 0
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 3 files compared)
Checked 12 log files, 9 edm output root files, 4 DQM output files
TriggerResults: found differences in 2 / 3 workflows

rappoccio · 2023-02-08T14:15:16Z

+1

cmsbuild added this to the CMSSW_13_0_X milestone Jan 12, 2023

cmsbuild added code-checks-pending heterogeneous-pending orp-pending pending-signatures tests-pending labels Jan 12, 2023

makortel mentioned this pull request Jan 12, 2023

Memory leaks when reading in PortableHostCollection #40491

Closed

cmsbuild added code-checks-approved and removed code-checks-pending labels Jan 12, 2023

cmsbuild added tests-started and removed tests-pending labels Jan 12, 2023

cmsbuild added tests-approved and removed tests-started labels Jan 12, 2023

makortel force-pushed the portableCollectionMemory branch from 44d861b to 8192b7f Compare January 12, 2023 14:45

cmsbuild added code-checks-pending tests-pending and removed tests-approved code-checks-approved labels Jan 12, 2023

cmsbuild added code-checks-approved and removed code-checks-pending labels Jan 12, 2023

cmsbuild added tests-approved and removed tests-started labels Feb 3, 2023

cmsbuild added fully-signed heterogeneous-approved and removed pending-signatures heterogeneous-pending labels Feb 8, 2023

cmsbuild added tests-started and removed tests-approved labels Feb 8, 2023

cmsbuild added tests-approved and removed tests-started labels Feb 8, 2023

cmsbuild added orp-approved and removed orp-pending labels Feb 8, 2023

cmsbuild merged commit 583e1a6 into cms-sw:master Feb 8, 2023

makortel deleted the portableCollectionMemory branch February 9, 2023 20:58

cmsbuild mentioned this pull request Feb 9, 2023

Update gcc 12 builds to CUDA 12.1 cms-sw/cmsdist#8296

Merged

makortel mentioned this pull request Nov 8, 2023

Implement ROOT read rules for PortableHostCollection #43219

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix memory leaks in PortableHostCollection deserialization #40492

Fix memory leaks in PortableHostCollection deserialization #40492

makortel commented Jan 12, 2023 •

edited

Loading

cmsbuild commented Jan 12, 2023

cmsbuild commented Jan 12, 2023

makortel commented Jan 12, 2023

makortel commented Jan 12, 2023

cmsbuild commented Jan 12, 2023

makortel commented Jan 12, 2023

cmsbuild commented Jan 12, 2023

cmsbuild commented Jan 12, 2023

makortel commented Jan 12, 2023

cmsbuild commented Feb 3, 2023

fwyzard commented Feb 4, 2023

fwyzard commented Feb 8, 2023

fwyzard commented Feb 8, 2023

cmsbuild commented Feb 8, 2023

cmsbuild commented Feb 8, 2023

rappoccio commented Feb 8, 2023

Fix memory leaks in PortableHostCollection deserialization #40492

Fix memory leaks in PortableHostCollection deserialization #40492

Conversation

makortel commented Jan 12, 2023 • edited Loading

PR description:

PR validation:

cmsbuild commented Jan 12, 2023

cmsbuild commented Jan 12, 2023

makortel commented Jan 12, 2023

makortel commented Jan 12, 2023

cmsbuild commented Jan 12, 2023

Comparison Summary

GPU Comparison Summary

makortel commented Jan 12, 2023

cmsbuild commented Jan 12, 2023

cmsbuild commented Jan 12, 2023

makortel commented Jan 12, 2023

cmsbuild commented Feb 3, 2023

Comparison Summary

GPU Comparison Summary

fwyzard commented Feb 4, 2023

fwyzard commented Feb 8, 2023

fwyzard commented Feb 8, 2023

cmsbuild commented Feb 8, 2023

cmsbuild commented Feb 8, 2023

Comparison Summary

GPU Comparison Summary

rappoccio commented Feb 8, 2023

makortel commented Jan 12, 2023 •

edited

Loading