-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Refactor handling of non-SOH generations #1688
Merged
Merged
Changes from all commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
22bbcda
refactoring
VSadov 71c7219
ploh-->uoh
VSadov d6761f0
fix Linux build
VSadov 326741b
some PR feedback
VSadov 7a5bf91
more PR feedback
VSadov b488709
more PR feedback
VSadov e8396d7
address comments in UpdatePreGCCounters
VSadov 1acd514
removed a confusing comment
VSadov f421d32
stress fix in background sweep
VSadov 536f3d1
use `saved_sweep_ephemeral_seg` in background sweep.
VSadov 59510de
fixed `GCPerHeapHistory_V3` event.
VSadov 1dba378
re-implemented https://github.com/dotnet/runtime/pull/2103 on top of …
VSadov File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we want to rename this allocBytesLoh field? this GetThreadAllocData is only used by sos I think (not by clrmd?)? @leculver @mikem8361 could you please confirm? for context we are renaming the alloc_bytes_loh field on the alloc_context to alloc_bytes_uoh because we are introducing a concept of uoh which includes loh and additional generations. #Resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like SOS doesn't use this API. Not sure about CLRMD. #Resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am suggesting changing this only when clients can use it. No benefit from changing this now, for sure, since there is still only loh in this change, but we may break something.
In reply to: 368189692 [](ancestors = 368189692)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We already have a tracking bug to revisit this - https://github.com/dotnet/coreclr/issues/27692
In reply to: 368208096 [](ancestors = 368208096,368189692)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For ClrMD we reimplement all of this interface in C#. Changing names won't affect us at all, but we require these structures do not change in layout or meaning for backwards compat.
When real changes go in that affect the underlying shape of the GC heap and/or requires changing the shape or usage of DAC data structures it's best to implement a new interface (ISOSDacX or similar) that we can query for to tell us there's a new style of heap involved.