-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
[android] stop using Build.VERSION.SdkInt #3783
Conversation
33d7adb
to
716cefe
Compare
public static bool IsAtLeast(BuildVersionCodes buildVersionCode) => OperatingSystem.IsAndroidVersionAtLeast((int)buildVersionCode); | ||
|
||
public static bool IsAtLeast(BuildVersionCodes buildVersionCode) | ||
{ | ||
return buildVersionCode >= BuildVersion; | ||
} | ||
public static bool IsAtLeast(int apiLevel) => OperatingSystem.IsAndroidVersionAtLeast(apiLevel); |
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.
NativeVersion.IsAtLeast()
and Supports()
are public, is it useful at all? Should we delete these methods?
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.
Discussed on a call, I'll mark these [Obsolete]
.
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.
The point of these was to have a cross-platform consistent way for us to do version checking and capabilities. That's why this is a partial class - so we can use these in handlers without having to use #ifdefs.
The intent was to wrap the OperatingSystem calls in the NativeVersion class: #461
Since we've shifted all the of the actual code in the handlers to platform-specific extension methods, maybe these aren't that useful? If that's the case, we don't need to obsolete them - we can just delete them. They're new with MAUI, and haven't seen a stable public release yet.
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.
The iOS one seems more useful, as it has this string capability
overload:
https://github.com/dotnet/maui/blob/main/src/Core/src/Platform/iOS/NativeVersion.cs
This PR is already getting huge -- so I think I'm going to leave these APIs here and just have them call OperatingSystem
on Android.
716cefe
to
2eb36ad
Compare
2eb36ad
to
f5b3dfb
Compare
We have a beautiful .NET 6 API we can use instead, that doesn't call into Java (no JNI!): https://docs.microsoft.com/dotnet/api/system.operatingsystem.isandroidversionatleast Usage of `Build.VERSION.SdkInt` came from both Xamarin.Forms and Xamarin.Essentials. We were caching the result of the value, but still doing that work twice. We can also use the pattern: OperatingSystem.IsAndroidVersionAtLeast((int)BuildVersionCodes.JellyBean) `BuildVersionCodes` is a regular C# enum, and so the C# compiler will strip this information away leaving just an integer in the final IL. I used either an integer or the enum, depending on what the existing code did. Additionally, I went through any existing code checking old API levels and removed it. The minimum API level for .NET 6 (dotnet/runtime & xamarin-android) is 21. So any code looking for API levels 21 or lower, could just be removed. I also removed code I found such as: public static int? TargetSdkVersion(this Context self) { return (int?)self?.ApplicationInfo?.TargetSdkVersion; } This was only used in one place, and was checking API 17, so I just removed it. ~~ Results ~~ Testing a `Release` build of `Maui.Controls.Sample.SingleProject.csproj` on a Pixel 5 device: Before: 12-16 14:41:19.289 1741 1935 I ActivityTaskManager: Displayed com.microsoft.maui.sample/crc64dac46b470c4e9200.MainActivity: +1s739ms 12-16 14:41:24.468 1741 1935 I ActivityTaskManager: Displayed com.microsoft.maui.sample/crc64dac46b470c4e9200.MainActivity: +1s713ms 12-16 14:41:29.867 1741 1935 I ActivityTaskManager: Displayed com.microsoft.maui.sample/crc64dac46b470c4e9200.MainActivity: +1s917ms 12-16 14:41:34.905 1741 1935 I ActivityTaskManager: Displayed com.microsoft.maui.sample/crc64dac46b470c4e9200.MainActivity: +1s765ms 12-16 14:41:40.132 1741 1935 I ActivityTaskManager: Displayed com.microsoft.maui.sample/crc64dac46b470c4e9200.MainActivity: +1s719ms 12-16 14:41:45.431 1741 1935 I ActivityTaskManager: Displayed com.microsoft.maui.sample/crc64dac46b470c4e9200.MainActivity: +1s768ms 12-16 14:41:50.608 1741 1935 I ActivityTaskManager: Displayed com.microsoft.maui.sample/crc64dac46b470c4e9200.MainActivity: +1s716ms 12-16 14:41:55.878 1741 1935 I ActivityTaskManager: Displayed com.microsoft.maui.sample/crc64dac46b470c4e9200.MainActivity: +1s754ms 12-16 14:42:01.157 1741 1935 I ActivityTaskManager: Displayed com.microsoft.maui.sample/crc64dac46b470c4e9200.MainActivity: +1s754ms 12-16 14:42:06.315 1741 1935 I ActivityTaskManager: Displayed com.microsoft.maui.sample/crc64dac46b470c4e9200.MainActivity: +1s713ms Average(ms): 1755.8 Std Err(ms): 19.1803139819046 Std Dev(ms): 60.6534784199921 After: 12-16 14:45:35.208 1741 1935 I ActivityTaskManager: Displayed com.microsoft.maui.sample/crc64dac46b470c4e9200.MainActivity: +1s737ms 12-16 14:45:40.448 1741 1935 I ActivityTaskManager: Displayed com.microsoft.maui.sample/crc64dac46b470c4e9200.MainActivity: +1s743ms 12-16 14:45:45.663 1741 1935 I ActivityTaskManager: Displayed com.microsoft.maui.sample/crc64dac46b470c4e9200.MainActivity: +1s715ms 12-16 14:45:50.931 1741 1935 I ActivityTaskManager: Displayed com.microsoft.maui.sample/crc64dac46b470c4e9200.MainActivity: +1s741ms 12-16 14:45:56.161 1741 1935 I ActivityTaskManager: Displayed com.microsoft.maui.sample/crc64dac46b470c4e9200.MainActivity: +1s712ms 12-16 14:46:01.426 1741 1935 I ActivityTaskManager: Displayed com.microsoft.maui.sample/crc64dac46b470c4e9200.MainActivity: +1s768ms 12-16 14:46:06.574 1741 1935 I ActivityTaskManager: Displayed com.microsoft.maui.sample/crc64dac46b470c4e9200.MainActivity: +1s699ms 12-16 14:46:11.867 1741 1935 I ActivityTaskManager: Displayed com.microsoft.maui.sample/crc64dac46b470c4e9200.MainActivity: +1s779ms 12-16 14:46:16.979 1741 1935 I ActivityTaskManager: Displayed com.microsoft.maui.sample/crc64dac46b470c4e9200.MainActivity: +1s694ms 12-16 14:46:22.229 1741 1935 I ActivityTaskManager: Displayed com.microsoft.maui.sample/crc64dac46b470c4e9200.MainActivity: +1s728ms Average(ms): 1731.6 Std Err(ms): 8.79924239163047 Std Dev(ms): 27.8256476414596 This appears to maybe save ~24ms on startup for this app?
f5b3dfb
to
b3ee9db
Compare
Description of Change
We have a beautiful .NET 6 API we can use instead, that doesn't call
into Java (no JNI!):
https://docs.microsoft.com/dotnet/api/system.operatingsystem.isandroidversionatleast
Usage of
Build.VERSION.SdkInt
came from both Xamarin.Forms andXamarin.Essentials. We were caching the result of the value, but still
doing that work twice.
We can also use the pattern:
BuildVersionCodes
is a regular C# enum, and so the C# compiler willstrip this information away leaving just an integer in the final IL.
I used either an integer or the enum, depending on what the existing
code did.
Additionally, I went through any existing code checking old API levels and removed it.
The minimum API level for .NET 6 (dotnet/runtime & xamarin-android) is 21. So any
code looking for API levels 21 or lower, could just be removed.
I also removed code I found such as:
This was only used in one place, and was checking API 17, so I just
removed it.
Results
Testing a
Release
build ofMaui.Controls.Sample.SingleProject.csproj
on a Pixel 5 device:
This appears to maybe save ~24ms on startup for this app?
PR Checklist
Does this PR touch anything that might affect accessibility?
No