-
Notifications
You must be signed in to change notification settings - Fork 554
[Mono.Android] trimmer feature switches for DotNetRuntimeType
#10296
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
||
[FeatureSwitchDefinition ($"{FeatureSwitchPrefix}{nameof (IsMonoRuntime)}")] | ||
internal static bool IsMonoRuntime { get; } = | ||
AppContext.TryGetSwitch ($"{FeatureSwitchPrefix}{nameof (IsMonoRuntime)}", out bool isEnabled) ? isEnabled : IsMonoRuntimeEnabledByDefault; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we should enforce that the feature switch is always set explicitly and throw if it isn't. I'm worried that we could introduce a regression and for example we would drop the Microsoft.Android.Runtime.RuntimeFeature.IsMonoRuntime
on CoreCLR in the SDK and now both IsMonoRuntime
and IsCoreClrRuntime
would be true.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll add something that throws on startup if both are set to true.
...ndroid.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.RuntimeConfig.targets
Outdated
Show resolved
Hide resolved
Context: #10198 (comment) `ManagedValueManager` is not currently trimmed away in Android apps running on Mono. We can make this happen by: * Remove `DotNetRuntimeType` enum * Introduce `RuntimeFeature.IsMonoRuntime` and `RuntimeFeature.IsCoreClrRuntime` * Set switches by default for each runtime: <ItemGroup> <RuntimeHostConfigurationOption Include="Microsoft.Android.Runtime.RuntimeFeature.IsMonoRuntime" Value="false" Trim="true" /> <RuntimeHostConfigurationOption Include="Microsoft.Android.Runtime.RuntimeFeature.IsCoreClrRuntime" Value="true" Trim="true" /> </ItemGroup> I have not yet seen a need within the code to introduce `RuntimeFeature.IsNativeAotRuntime`, but we could do so in the future. Currently, NativeAOT works via the other two runtime feature switches being `false` and then none of the problematic code paths are hit.
f615d39
to
d834c56
Compare
"PackageSize": 3148309 | ||
"PackageSize": 3090965 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like it solves the minor app size regression from:
The number we used to have was 3078677
, but that could have moved around slightly from other changes.
Context: #10198 (comment)
ManagedValueManager
is not currently trimmed away in Android apps running on Mono.We can make this happen by:
Remove
DotNetRuntimeType
enumIntroduce
RuntimeFeature.IsMonoRuntime
andRuntimeFeature.IsCoreClrRuntime
Set switches by default for each runtime:
I have not yet seen a need within the code to introduce
RuntimeFeature.IsNativeAotRuntime
, but we could do so in the future.Currently, NativeAOT works via the other two runtime feature switches being
false
and then none of the problematic code paths are hit.