Enable GC Regions on Mac #115251
Enable GC Regions on Mac #115251
Conversation
ghost
added
the
mangod9
added
the
NO-MERGE
|
Tagging subscribers to this area: @dotnet/gc |
|
It is ok to try and the CI may even pass if the run does not happen to generate crash dump along the way. We need a positive confirmation that enabling regions GC won't kill post-mortem diagnostic on mac. Have we done anything to resolve the problem with prohibitively large crash dumps on mac? |
|
the last time we looked we werent able to repro the large dumps locally. Believe @janvorli had tried it too, given lower priority we never investigated it completely. Hoping we get to root cause this time around. |
|
I have validated that with large memory reservation size with regions the dump generated on MacOS is about 100mb larger (so it's only about 2% more) -- not order of magnitude larger we thought it would be. Need to figure out failures in iOS/tvOS NativeAOT failures before we can enable. |
|
excluding iOS and tvOS seems to get things to work. Will need to figure out if reducing the reservation size would get regions to work on those platforms too. |
mangod9
changed the title
| @@ -140,11 +140,11 @@ 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 and macOS builds | |||
| // For now disable regions for standalone GC and iOS and tvOS builds | |||
There was a problem hiding this comment.
| // For now disable regions for standalone GC and iOS and tvOS builds | |
| // For now disable regions for iOS and tvOS builds |
might as well take this opportunity to fix this comment since we do have a standalone GC with regions.
| @@ -22,7 +22,7 @@ static int Main() | |||
|
|
|||
| long lowerBound, upperBound; | |||
| lowerBound = 1200 * 1024; // ~1.2 MB | |||
| upperBound = 1600 * 1024; // ~1.6 MB | |||
| upperBound = 1700 * 1024; // ~1.7 MB | |||
There was a problem hiding this comment.
what's the reason for this change?
There was a problem hiding this comment.
this test checks for size_on_disk of the test binary. Regions adds more code which puts it over the test threshold.
I do not think this is a good plan. Analyzing failures and debugging iOS/tvOS is much harder than debugging macOS. If a segments-specific issue gets introduced in the meantime, it will be a lot harder and much more time consuming to diagnose it. |
|
ok, will investigate failures and enable for them too. |
Regions will be enabled for macOS, but still disabled for iOS and tvOS builds for now (will investigate the test failures on those platforms and try to enable in subsequent PRs).