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

Optionally capture a dump during generational aware analysis #54517

Merged
merged 3 commits into from
Jun 30, 2021

Conversation

cshung
Copy link
Member

@cshung cshung commented Jun 21, 2021

This change introduces an extra option to allow generational aware analysis to capture a dump in addition to the trace files.

The trace file contains information about the type of object that held the young objects alive, but very often, we also need to know the exact instance. This is particularly useful when the young objects are leaked by a delegate, where we would like to know which callback is causing the problem.

@dotnet/dotnet-diag

@ghost
Copy link

ghost commented Jun 21, 2021

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

Issue Details

This change introduces an extra option to allow generational aware analysis to capture a dump in addition to the trace files.

The trace file contains information about the type of object that held the young objects alive, but very often, we also need to know the exact instance. This is particularly useful when the young objects are leaked by a delegate, where we would like to know which callback is causing the problem.

@dotnet/dotnet-diag

Author: cshung
Assignees: -
Labels:

area-GC-coreclr

Milestone: -

Copy link
Member

@noahfalk noahfalk left a comment

Choose a reason for hiding this comment

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

Looks fine to me, @mikem8361 you want to take a look as well?

@cshung
Copy link
Member Author

cshung commented Jun 30, 2021

@Maoni0, can you please approve the PR if the change is good from the GC's perspective?
@noahfalk, can you please take a look again? I made some refactoring to the underlying code since your last approval. The refactoring should make it easier to capture a dump without having to have an ifdef at the call site.

@Maoni0
Copy link
Member

Maoni0 commented Jun 30, 2021

looks good from GC's POV.

Copy link
Member

@noahfalk noahfalk left a comment

Choose a reason for hiding this comment

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

Still looks good to me : )
(And thanks for the little refactor on GenerateDump)

Copy link
Member

@hoyosjs hoyosjs left a comment

Choose a reason for hiding this comment

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

Dump changes and EP changes look like a cleanup to me :)

@cshung cshung merged commit 8818c15 into dotnet:main Jun 30, 2021
@cshung cshung deleted the public/gen-aware-dump branch June 30, 2021 20:20
@ghost ghost locked as resolved and limited conversation to collaborators Jul 30, 2021
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