Commits on Jan 7, 2023

1. [generator] enum map.csv can set @deprecated-since for enum values (#… …

   …1070)
   
   Context: dotnet/android#7670
   
   While auditing the `Mono.Android.dll`, we've found that some constants
   were added to the wrong enums.
   
   Consider [`xamarin-android/src/Mono.Android/map.csv`][0],
   lines 739-743:
   
   	E,28,android/app/usage/UsageStatsManager.STANDBY_BUCKET_ACTIVE,10,Android.App.Usage.StandbyBucket,Active,remove,
   	E,28,android/app/usage/UsageStatsManager.STANDBY_BUCKET_FREQUENT,30,Android.App.Usage.StandbyBucket,Frequent,remove,
   	E,28,android/app/usage/UsageStatsManager.STANDBY_BUCKET_RARE,40,Android.App.Usage.StandbyBucket,Rare,remove,
   	E,30,android/app/usage/UsageStatsManager.STANDBY_BUCKET_RESTRICTED,45,Android.App.Usage.UsageStatsInterval,Restricted,remove,
   	E,28,android/app/usage/UsageStatsManager.STANDBY_BUCKET_WORKING_SET,20,Android.App.Usage.StandbyBucket,WorkingSet,remove,
   
   
   `STANDBY_BUCKET_RESTRICTED` should have been added to `StandbyBucket`
   instead of `UsageStatsInterval`.
   
   To fix this in a backwards-compatible way, we can add it to both enums:
   
   	E,30,android/app/usage/UsageStatsManager.STANDBY_BUCKET_RESTRICTED,45,Android.App.Usage.StandbyBucket,Restricted,remove,
   	A,30,,45,Android.App.Usage.UsageStatsInterval,Restricted,remove,
   
   However, we have no way to mark `UsageStatsInterval.Restricted` as
   `[Obsolete]`, so people may still unintentionally use it.
   
   Add a new field to our csv that allows us to set a constant's
   `@deprecated-since` field, the `30` at the end:
   
   	A,30,,45,Android.App.Usage.UsageStatsInterval,Restricted,remove,,30
   
   This allows us to generate:
   
   	[global::System.Runtime.Versioning.ObsoletedOSPlatform ("android30.0")]
   	Restricted = 45
   
   Additionally, when the cause is that we accidentally added an invalid
   value to the enum, we should provide a better obsolete message, letting
   the user know that the value will not function.  This is done by
   setting `@deprecated-since` to `-1`:
   
   	A,30,,45,Android.App.Usage.UsageStatsInterval,Restricted,remove,,-1
   
   resulting in:
   
   	[global::System.Obsolete (@"This value was incorrectly added to the enumeration and is not a valid value")]
   	Restricted = 45
   
   [0]: https://github.com/xamarin/xamarin-android/blob/cc70ce20aff6934532a8cb6a7bddbf3710fd7e1c/src/Mono.Android/map.csv

   

   jpobst authored Jan 7, 2023
   Configuration menu

   • View commit details
   • Copy full SHA for 5c5dc08
   • Browse repository at this point

   Copy the full SHA

   5c5dc08 View commit details

   Browse the repository at this point in the history

2. [Java.Interop.Tools.JavaCallableWrappers] use IMetadataResolver more (#… …

   …1069)
   
   Context: b81cfbb
   
   Reviewing code in `JavaCallableWrapperGenerator`, I found codepaths
   where we weren't caching `TypeReference.Resolve()` calls *at all*,
   e.g. `JavaNativeTypeManager.GetPrimitiveClass(TypeDefinition)`.
   We thus appear to be calling `TypeReference.Resolve()` potentially on
   the same types.
   
   For example, `dotnet trace` output of an incremental build of a `dotnet new maui` project:
   
   	41.65ms xamarin.android.cecil!Mono.Cecil.TypeReference.Resolve()
   
   It appears that, in trying to make our maintenance lives easier by
   preserving backward API compatibility with callers that couldn't
   provide a `TypeDefinitionCache` or `IMetadataResolver` instance -- by
   providing method overloads which took e.g.
   `IMetadataResolver? resolver` parameters which could be `null` -- we
   made our optimization and performance lives *harder*, because not
   all codepaths which needed to use caching *were* using caching, as
   they were overlooked.
   
   As the `Java.Interop.Tools.*` assemblies are internal API, introduce
   an *API break* while preserving *ABI*:
   
   `IMetadataResolver` is now required.
   
   Thus, previous/current "compatibility methods" which allow *not*
   providing an `IMetadataResolver` instance such as:
   
   	// old and busted
   	partial class TypeDefinitionRocks {
   	    [Obsolete ("Use the TypeDefinitionCache overload for better performance.")] 
   	    public static TypeDefinition? GetBaseType (this TypeDefinition type) => GetBaseType (type, resolver: null); 
   	}
   
   will now become *errors* to use:
   
   	// new hawtness
   	partial class TypeDefinitionRocks {
   	    [Obsolete ("Use the TypeDefinitionCache overload for better performance.", error:true)] 
   	    public static TypeDefinition? GetBaseType (this TypeDefinition type) => throw new NotSupportedException ();
   	}
   
   This allows the C# compiler to help us audit our codebase, ensuring
   that *all* codepaths which call `TypeReference.Resolve()` instead use
   `IMetadataResolver.Resolve()`, including:
   
     * `JavaCallableWrapperGenerator.GetAnnotationsString()`
     * `JavaCallableWrapperGenerator.WriteAnnotations()`
     * `JavaNativeTypeManager.GetPrimitiveClass()`
   
   Requiring caching results in:
   
   	23.89ms xamarin.android.cecil!Mono.Cecil.TypeReference.Resolve()
   
   Additionally, I updated two places to use plain `foreach` loops
   instead of using System.Linq.
   
     * Before:
   
           1.03s xamarin.android.build.tasks!Xamarin.Android.Tasks.GenerateJavaStubs.RunTask()
   
     * After:
   
           944.48ms xamarin.android.build.tasks!Xamarin.Android.Tasks.GenerateJavaStubs.RunTask()
   
   I think this likely saves about ~50ms off incremental builds of a
   `dotnet new maui` project.

   

   jonathanpeppers authored Jan 7, 2023
   Configuration menu

   • View commit details
   • Copy full SHA for cf80deb
   • Browse repository at this point

   Copy the full SHA

   cf80deb View commit details

   Browse the repository at this point in the history