[release/8.0] Delete the RequiresPreviewFeaturesAttribute from RefreshMemoryLimit

[cshung]

Backport of #91622 to release/8.0

Customer Impact

This is to enable the RefreshMemoryLimit feature so that it won't require the RequiresPreviewFeaturesAttribute. This is going to be used by Amazon Lambda here.

Testing

The internal working is tested by enabling the COMMITTED_BYTES_SHADOW feature switch - the switch will trigger a call to RefreshMemoryLimit for each GC, and it was run as part of the GC exit criteria run.

The managed part of the method is not currently tested, work is in progress to provide some coverage through #91833.

Risk

The API is already public, this change does not add any more risk for customers who will call it regardless of the existence of a warning or not.

[dotnet-issue-labeler]

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

[ghost]

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

Issue Details

---

null

Author:	cshung
Assignees:	cshung
Labels:	area-System.Runtime, new-api-needs-documentation, needs-area-label
Milestone:	-

[stephentoub]

Are there tests somewhere for GC.RefreshMemoryLimit()? I'm not seeing any.

[cshung]

Are there tests somewhere for GC.RefreshMemoryLimit()? I'm not seeing any.

If we turn on #define COMMITTED_BYTES_SHADOW, this will stress test the RefreshMemoryLimit() before every GC. We tested it that way.

[stephentoub]

If we turn on #define COMMITTED_BYTES_SHADOW, this will stress test the RefreshMemoryLimit() before every GC. We tested it that way.

It calls the managed method, or the underlying functionality? If I were to change the managed method to nop or throw accidentally, what test would fail? And if the answer is "none", that's ok?

[cshung]

If we turn on #define COMMITTED_BYTES_SHADOW, this will stress test the RefreshMemoryLimit() before every GC. We tested it that way.

It calls the managed method, or the underlying functionality? If I were to change the managed method to nop or throw accidentally, what test would fail? And if the answer is "none", is that ok?

You are right, if someone nop the method, no test will detect that failure.

From a risk perspective, it is so much easier for a GC dev to forget hard limit could be changed and therefore write incompatible code than someone nop the managed method, that's why testing has been focused there. There are some glue code at the qcall layer that won't be exercised without a test calling from the managed side as well.

I think there is value to add a managed test, let me add one.

[carlossanlop]

@cshung can you please fill out the servicing template from the PR description and get an M2 approval when this is ready to merge?

[jeffschwMSFT]

approved. this can be merged when ready