[CoreCLR] Limit ReadyToRun generic cycle expansion

[simonrozsival]

Summary

• default Android CoreCLR ReadyToRun builds to --maxgenericcycle:2 and --maxgenericcyclebreadth:1
• preserve explicit customer overrides for either option, including command-line global properties and colon, equals, or separate-value syntax
• add focused MSBuild coverage for the defaults and override combinations

Motivation

Recursive value-type generic instantiations can cause crossgen2 to produce disproportionately large native images. In the investigation related to dotnet/runtime#134182, the conservative limits removed about 95% of the cycle-specific growth on both Android and iOS.

A scaled Android arm64 repro produced a 400.7 MB intermediate R2R image and 66.96 MB APK with the existing defaults, compared with an 18.65 MB .NET 10 Mono APK from the same source.

The limits had no observable cost on dotnet new maui --sample-content using MAUI RC1:

Metric	Existing defaults	New limits
Android arm64 APK	23,873,615 bytes	23,873,615 bytes
Android composite R2R	5,574,144 bytes	5,574,144 bytes
iOS arm64 IPA	23,151,364 bytes	23,151,365 bytes
iOS composite R2R	24,631,024 bytes	24,631,024 bytes
Samsung A16 startup median	1,877 ms	1,902 ms
Samsung A16 startup p90	1,925 ms	1,933 ms

The startup benchmark used 30 cold process starts per configuration in ABBA order. Block-to-block drift was larger than the aggregate difference; the second baseline and limited blocks had means of 1,901.0 ms and 1,901.3 ms respectively.

This is a defensive default: applications whose generic graph does not reach the cutoffs produce the same native image size, while pathological recursive expansion is bounded.

Validation

• ReadyToRunGenericCycleLimits: 9/9 passed
• built Xamarin.Android.Build.Tests
• validated the target as XML and with standalone MSBuild probes for all supported override forms
• benchmarked the RC1 MAUI sample-content app on a Samsung Galaxy A16 (SM-A165F, Android 16)

Related investigation: https://gist.github.com/simonrozsival/423d3cbd0c1719433e2eacf4d8314820

[Copilot]

Copilot review overview

🟢 Approval recommended

All supplied approval-readiness assessments indicate readiness; remaining comments concern test coverage and assertion strength.

Review effort: Lite
Findings: None

What changed in this PR

Adds Android CoreCLR ReadyToRun generic-cycle limits while preserving explicit crossgen2 overrides and expanding MSBuild coverage.

Changes:

• Applies conservative cycle and breadth defaults.
• Supports command-line override syntaxes.
• Adds focused default and override tests.

File	Summary	Final review comments
src/​Xamarin.Android.Build.Tasks/​Tests/​Xamarin.Android.Build.Tests/​BuildTest2.cs	Tests default and override behavior.	Moderate, 1 vote: Add negative cases for Android and ReadyToRun gates. Nit, 1 vote: Test whitespace-separated values. Nit, 1 vote: Assert argument sequence ordering.
src/​Xamarin.Android.Build.Tasks/​Microsoft.Android.Sdk/​targets/​Microsoft.Android.Sdk.CoreCLR.targets	Applies generic-cycle defaults for Android CoreCLR ReadyToRun builds.	No comments.

[BrzVlad]

Did this provide any benefit on a real application, or was it tested only on an artificial repro, that might not be representative ?

[jonathanpeppers]

AI keeps putting this TreatAsLocalProperty value for me, too, but it should just stop doing that.

Can we update copilot-instructions.md to say that we don't care about global property override behavior (that's default MSBuild!) and most of the time TreatAsLocalProperty is not needed.

[jonathanpeppers]

Is there a way to do this without a bunch of inlined regex?

Should we make a C# task for modifying $(PublishReadyToRunCrossgen2ExtraArgs), that could parse them and reformat them?

[simonrozsival]

sure, we can do that. let's first agree if we even want this as the default and if yes, I'll update the PR.

[simonrozsival]

Did this provide any benefit on a real application

@BrzVlad No, I was trying to reproduce the R2R image explosion reported in dotnet/runtime#134182 and I found that with recursive generic data structures the R2R image grows quite fast while producing small size with Mono AOT:

static int Expand<T> (T value, int depth) where T : struct
{
	if (depth == 0) {
		return EqualityComparer<T>.Default.GetHashCode (value);
	}

	return Expand (new Left<T> (value), depth - 1) +
		Expand (new Right<T> (value), depth - 1);
}

readonly record struct Left<T> (T Value) where T : struct;
readonly record struct Right<T> (T Value) where T : struct;

(see the gist in the description for more details)

This should not have negative impact on most apps, so I thought it might not be a bad idea to set this as a cautious default.

[BrzVlad]

I'm not necessarily against this change, we probably care more about R2R size compared to desktop consumers so it makes sense. I just believe, in general, it is a good principle to avoid changing defaults unless we have a clear customer driven scenario showing its usefullness. Otherwise we are doing changes that might have no impact, while each change can have an unforeseen risk attached to it.

Also, I was considering for the mobile build to respect the OptimizationPreference project level attribute, so maybe it makes sense to export this at a higher level, in the sdk and have the same r2r configuration used by iOS as well. cc @vitek-karas

[jkotas]

This should not have negative impact on most apps

Excessive or recursive generics expansion is always a performance problem. Most apps and libraries are well written and avoid this performance problem in the first place.

If the app has this performance problem, it can result into large AOT image, poor performance at runtime, or both. These configuration switches allow one to choose the manifestation of the performance problem as a workaround. There is no universal "right" setting. For example, it should be possible to write a synthetic test where the default overrides introduced in this PR introduce a runtime performance regression.