Skip to content

Conversation

@github-actions
Copy link
Contributor

@github-actions github-actions bot commented Dec 27, 2022

Backport of #79853 to release/7.0, issue #71472

Creating a dump against some kernel implementations is committing empty pages with the page probing technique we currently use during memory enumeration. This change uses the pagemap API's in the /proc system if it's available along with the maps api to decide what pages contain relevant information. This results in smaller dumps and no increase in resident memory usage.

/cc @hoyosjs @ezsilmar

Customer Impact

Customers have reported memory doubling in processes they dump. This is fatal environments such as K8s and cgroups hosting, where the process of collecting a dump often results of a OOM kill. This makes crash and ad-hoc diagnostics harder than needs be.

Testing

TBD

Risk

Low - fallback to prior code paths is enabled. Also, it's possible to explicitly disable the new behavior by setting the env var DOTNET_DbgDisablePagemapUse to 1.

@ghost
Copy link

ghost commented Dec 27, 2022

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

Issue Details

Backport of #79853 to release/7.0

/cc @hoyosjs @ezsilmar

Customer Impact

Testing

Risk

IMPORTANT: Is this backport for a servicing release? If so and this change touches code that ships in a NuGet package, please make certain that you have added any necessary package authoring and gotten it explicitly reviewed.

Author: github-actions[bot]
Assignees: -
Labels:

area-Diagnostics-coreclr

Milestone: -

@hoyosjs hoyosjs requested a review from mikem8361 December 27, 2022 00:38
@hoyosjs hoyosjs added the Servicing-consider Issue for next servicing release review label Dec 27, 2022
@hoyosjs hoyosjs added this to the 7.0.x milestone Dec 27, 2022
@hoyosjs hoyosjs force-pushed the backport/pr-79853-to-release/7.0 branch from 90b2ffd to 40cb952 Compare December 28, 2022 01:42
@jeffschwMSFT jeffschwMSFT removed the Servicing-consider Issue for next servicing release review label Jan 3, 2023
@mikem8361 mikem8361 self-assigned this Jan 3, 2023
@jeffschwMSFT jeffschwMSFT added the Servicing-consider Issue for next servicing release review label Jan 3, 2023
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. we will take for consideration in 7.0.x

@hoyosjs hoyosjs added the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Jan 5, 2023
@jeffschwMSFT jeffschwMSFT removed the Servicing-consider Issue for next servicing release review label Jan 10, 2023
@hoyosjs hoyosjs marked this pull request as draft January 12, 2023 23:27
ezsilmar and others added 4 commits February 10, 2023 14:26
Dumping memory regions as they are listed in /proc/pid/maps
results in increase of RAM usage of the target application
on some Linux kernels.

This change uses /proc/pid/pagemap to check if the page is committed
before adding it to the regions list. As the file is not available on
kernels 4.0 and 4.1 without elevated permissions there's a fallback to
previous behavior.
@carlossanlop carlossanlop force-pushed the backport/pr-79853-to-release/7.0 branch from 40cb952 to 36b5626 Compare February 10, 2023 22:26
@carlossanlop
Copy link
Contributor

This is still marked as no-merge. Friendly reminder that the servicing branches open today.

@carlossanlop
Copy link
Contributor

I'm retargeting this PR to the new release/7.0-staging branch, which is the one that we will use from now on for servicing fixes.

Repo maintainers will now be allowed to merge their own servicing PR as long as it meets the requirements:

  • It is appoved by Tactics (signaled by adding the Servicing-approved label).
  • It's signed-off by an area owner.
  • The CI is green, or the failures are investigated as unrelated.
  • And if the PR touches an OOB package, the necessary OOB authoring changes are added.

The new process is described here: runtime/docs/project/library-servicing.md.

The infra team will be actively monitoring servicing PRs to ensure all requirements are met and to help with any issues.

Let me know if you have any questions.

@carlossanlop carlossanlop changed the base branch from release/7.0 to release/7.0-staging March 28, 2023 20:54
@mikem8361 mikem8361 closed this Apr 17, 2023
@jkotas jkotas deleted the backport/pr-79853-to-release/7.0 branch April 18, 2023 22:46
@ghost ghost locked as resolved and limited conversation to collaborators May 19, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-Diagnostics-coreclr NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants