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.

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

[release/7.0] Add faster DAC EnumMemoryRegion option with less memory usage (#74300) #74464

Merged
merged 1 commit into from
Aug 24, 2022

Conversation

mikem8361
Copy link
Member

Customer Impact

Our partner team reports that on Linux, createdump can take up to 150mb of memory usage in a Linux container environment and may cause the container to OOM.

Issue: #72148

Details

Instead of drilling down into all the individual MT/MD/EEClass, etc. data structures, add the LoaderAllocator/LoaderHeaps regions directly.

Add new CLRDATA_ENUM_MEM_HEAP2 flag for the fast path.

To reduce risk of incomplete core dumps this is enabled by the COMPlus_EnableFastHeapDumps env var. This env var is only looked at by the Linux/MacOS createdump. It is currently ignored on Windows. The new HEAP2 flag is works when passed to the EnumMemoryRegions API on Windows but createdump can't set it because MiniDumpWriteDump in dbghelp.dll loads/calls the DAC API.

Fix MacOS dlopen error message (minor createdump logging change for VS4Mac).

Testing

Local and SOS testing.

Risk

Low risk. Affects only the DAC under an opt-in env var.

…#74300)

Issue: dotnet#72148

Instead of drilling down into all the individual MT/MD/EEClass, etc. data structures, add the LoaderAllocator/LoaderHeaps regions directly.

Add new CLRDATA_ENUM_MEM_HEAP2 flag for the fast path.

To reduce risk of incomplete core dumps this is enabled by the COMPlus_EnableFastHeapDumps env var. This env var is only looked at by the Linux/MacOS createdump. It is currently ignored on Windows. The new HEAP2 flag is works when passed to the EnumMemoryRegions API on Windows but createdump can't set it because MiniDumpWriteDump in dbghelp.dll loads/calls the DAC API.

Fix MacOS dlopen error message
@ghost
Copy link

ghost commented Aug 23, 2022

Tagging subscribers to this area: @tommcdon
See info in area-owners.md if you want to be subscribed.

Issue Details

Customer Impact

Our partner team reports that on Linux, createdump can take up to 150mb of memory usage in a Linux container environment and may cause the container to OOM.

Issue: #72148

Details

Instead of drilling down into all the individual MT/MD/EEClass, etc. data structures, add the LoaderAllocator/LoaderHeaps regions directly.

Add new CLRDATA_ENUM_MEM_HEAP2 flag for the fast path.

To reduce risk of incomplete core dumps this is enabled by the COMPlus_EnableFastHeapDumps env var. This env var is only looked at by the Linux/MacOS createdump. It is currently ignored on Windows. The new HEAP2 flag is works when passed to the EnumMemoryRegions API on Windows but createdump can't set it because MiniDumpWriteDump in dbghelp.dll loads/calls the DAC API.

Fix MacOS dlopen error message (minor createdump logging change for VS4Mac).

Testing

Local and SOS testing.

Risk

Low risk. Affects only the DAC under an opt-in env var.

Author: mikem8361
Assignees: mikem8361
Labels:

area-Diagnostics-coreclr

Milestone: -

Copy link
Contributor

@leculver leculver left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume performance tracing showed that this was what was causing the slowdown. Do we have a performance test before/after? How much time does this save overall?

The best way to get test coverage of this is to simply run all SOS commands before/after this change and make sure nothing breaks. Since this has a lot to do with loader heaps, commands like !eeheap, !dumpdomain, !dumpmodule, !dumpclass, !name2ee, and the basics of !threads, !dumpheap, and !verifyheap should cover everything...IIRC.

src/coreclr/inc/clrdata.idl Show resolved Hide resolved
@leculver
Copy link
Contributor

With respect to #72148, does using this new flag reduce overall memory usage? In addition to my question on how much faster this is, is there a comparison of peak to regular memory usage?

Sorry to hoist additional perf questions here, but I wanted to double-check the memory and wall-time improvements before calling related issues fixed when we report back to partner teams.

@mikem8361
Copy link
Member Author

In my memory usage profiling (via Valgrind) against my "BigApp" test app with 100's of assemblies, 30 or so types and methods in each assembly (something like a large real world app) went from memory usage of 300MB to almost no memory. Performance wise it went from minutes to generate the dump to secs.

This change saves on both createdump/DAC memory usage and wall clock time to generate the dump.

@carlossanlop carlossanlop changed the title Add faster DAC EnumMemoryRegion option with less memory usage (#74300) [release/7.0] Add faster DAC EnumMemoryRegion option with less memory usage (#74300) Aug 24, 2022
Copy link
Member

@jeffschwMSFT jeffschwMSFT left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

approved

@carlossanlop
Copy link
Member

Approved and signed off.
CI is green.
Ready to merge. :shipit:

@carlossanlop carlossanlop merged commit d8cc63f into dotnet:release/7.0 Aug 24, 2022
@mikem8361 mikem8361 deleted the enummem70 branch August 24, 2022 17:13
@ghost ghost locked as resolved and limited conversation to collaborators Sep 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants