Decommit memory before returning to the region_allocator

[cshung]

When we decommit_region, we wanted to also de-commit the associated mark array. The method decommit_mark_array_by_seg requires a gc_heap instance, so I just used the one stored on the region.

For regions that were returned to the free list earlier, their heaps were reset to null, so we will be using null to de-commit the mark array, and that will fail.

Instead of figuring out what heap it was. It is easier to recognize that in the case of USE_REGIONS, the mark_array is always the same across all heaps, and the partially committed mark array case just won't happen with USE_REGIONS. Therefore, we can simply pick an arbitrary heap and use it to de-commit the mark array.

[ghost]

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

Issue Details

---

This reverts commit 6da8e2a.

Author:	cshung
Assignees:	cshung
Labels:	area-GC-coreclr
Milestone:	-

[eerhardt]

Is there any way to add tests for this? So that we don't regress in this area again. It took a bit of time just to diagnose this issue in ASP.NET's CI.

[sebastienros]

Looks like this was a "revert PR" that was later changed to a fix. I would have preferred the previous PR to be merged as-is to unblock the aspnet CI and the benchmarks while the fix is being worked on.

[sebastienros]

#80640

[kasperk81]

@cshung or @jkotas can you merge this pr to unblock dotnet/aspnetcore#46055. aspnetcore is blocking sdk and installer

[jkotas]

I won't merge this without a sign-off from @Maoni0 or somebody else from the GC team.

I can merge revert of the PR that introduced the break. I agree with @sebastienros that it should have been the first course of action: #80723

[jkotas]

I can merge revert of the PR that introduced the break. I agree with @sebastienros that it should have been the first course of action: #80723

Done.

[Maoni0]

I was waiting on Andrew to address the incorrect comment. I see that @jkotas already reverted the PR. thanks Jan.

[cshung]

/backport to release/7.0

[github-actions]

Started backporting to release/7.0: https://github.com/dotnet/runtime/actions/runs/4027922454

[github-actions]

@cshung backporting to release/7.0 failed, the patch most likely resulted in conflicts:

$ git am --3way --ignore-whitespace --keep-non-patch changes.patch

Applying: Decommit memory as needed (#79912)
Using index info to reconstruct a base tree...
M	src/coreclr/gc/gc.cpp
M	src/coreclr/gc/gcpriv.h
Falling back to patching base and 3-way merge...
Auto-merging src/coreclr/gc/gcpriv.h
Auto-merging src/coreclr/gc/gc.cpp
CONFLICT (content): Merge conflict in src/coreclr/gc/gc.cpp
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 Decommit memory as needed (#79912)
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Error: The process '/usr/bin/git' failed with exit code 128

Please backport manually!

[github-actions]

@cshung an error occurred while backporting to release/7.0, please check the run log for details!

Error: git am failed, most likely due to a merge conflict.

[eerhardt]

Is there any way to add tests for this? So that we don't regress in this area again. It took a bit of time just to diagnose this issue in ASP.NET's CI.

Was there no way to write tests for the regression?

[cshung]

Was there no way to write tests for the regression?

There is no easy way to accomplish that regression testing within the runtime CI, but we have established new processes to make sure GC PRs are validated against the ASP.NET Benchmarks for functional and perf validation. @mrsharm is building automation around that, and this PR is tested using it.