Annotation changes only (part 3) #82697
Merged
Conversation
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
Member
Maoni0
commented
Feb 27, 2023
•
Maoni0
commented
- moved the per heap/global fields to the last part of the gc_heap class so it's much easier to read them
- made members that should be private actually private, so almost all of the fields are private now (instead of public)
- changed the bookkeeping fields from MAINTAINED to INIT_ONLY to be more appropriate for regions
- moved reset_mm_p into gc_heap class.
- got rid of the frozen_object_p method - this shouldn't have been added, there's already an API on the GCHeap interface that provides this info
- there are members that really don't need to be public but I'm trying to avoid too many kinds of changes at a time so I left them as public such as gc_started/wait_for_gc_done()/region_info enum. things like WaitLonger/stomp_write_barrier_ephemeral/reserved_memory_limit should really just be gc_heap methods
+ made members that should be private actually private + changed the bookkeeping fields from MAINTAINED to INIT to be more appropriate for regions + moved reset_mm_p into gc_heap class. + got rid of the frozen_object_p method - this shouldn't have been added, there's already an API on the GCHeap interface that provides this info + there are members that really don't need to be public but I'm trying to avoid too many kinds of changes at a time so I left them as public such as gc_started/wait_for_gc_done()/region_info enum. things like WaitLonger/stomp_write_barrier_ephemeral/reserved_memory_limit should really just be gc_heap methods
ghost
assigned 
PeterSolMS
reviewed
Feb 27, 2023
src/coreclr/gc/gcpriv.h
Outdated
| @@ -131,7 +131,7 @@ inline void FATAL_GC_ERROR() | |||
| // This means any empty regions can be freely used for any generation. For | |||
| // Server GC we will balance regions between heaps. | |||
| // For now disable regions for StandAlone GC, NativeAOT and MacOS builds | |||
| #if defined (HOST_64BIT) && !defined (BUILD_AS_STANDALONE) && !defined(__APPLE__) | |||
| #if defined (BUILD_AS_STANDALONE) && defined (HOST_64BIT) && !defined(__APPLE__) | |||
There was a problem hiding this comment.
This looks like a temporary change to flip clrgc.dll to regions...
There was a problem hiding this comment.
ooh thanks! forgot to flip it back.
PeterSolMS
reviewed
Feb 27, 2023
src/coreclr/gc/gcpriv.h
Outdated
| @@ -1323,7 +1327,7 @@ class mark_queue_t | |||
| }; | |||
|
|
|||
| //class definition of the internal class | |||
| class gc_heap | |||
| class gc_heap // | |||
There was a problem hiding this comment.
There's a // starting a comment here, but no comment?
There was a problem hiding this comment.
that's also temporary (just to make searching easier). will get rid of it! when I did the final passes on the code I only concentrated on the field sections so missed these 😥
PeterSolMS
approved these changes
Feb 28, 2023
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.