[Xamarin.Android.Build.Tasks] parse JDK release file directly
#8663
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fixes: dotnet#8369 During Android project builds, the `<ValidateJavaVersion/>` MSBuild task currently shells out to `java -version` and `javac -version` to determine if we have a *valid* & usable JDK. Unfortunately, some customer logs have reported build times that are completely bananas: Task ValidateJavaVersion 23.469s ... Top 10 most expensive tasks ... ValidateJavaVersion = 3:46.740, 35 calls `java -version` and/or `javac -version` must be quite slow! Reading the source for OpenJDK, it appears that `java -version` actually starts a JVM to call a single method: * https://github.com/openjdk/jdk/blob/df370d725e5ae55a05479e8375bf665233ac3e44/src/java.base/share/native/libjli/java.c#L1895-L1913 * https://github.com/openjdk/jdk/blob/c9cacfb25d1f15c879c961d2965a63c9fe4d9fa7/src/java.base/share/classes/java/lang/VersionProps.java.template#L191-L233 That could explain the slowness? To improve this, let's just parse the file: * C:\Program Files\Microsoft\jdk-11.0.19.7-hotspot\release With contents: IMPLEMENTOR="Microsoft" IMPLEMENTOR_VERSION="Microsoft-7621296" JAVA_VERSION="11.0.19" JAVA_VERSION_DATE="2023-04-18" LIBC="default" MODULES="java.base java.compiler java.datatransfer java.xml java.prefs java.desktop java.instrument java.logging java.management java.security.sasl java.naming java.rmi java.management.rmi java.net.http java.scripting java.security.jgss java.transaction.xa java.sql java.sql.rowset java.xml.crypto java.se java.smartcardio jdk.accessibility jdk.internal.vm.ci jdk.management jdk.unsupported jdk.internal.vm.compiler jdk.aot jdk.internal.jvmstat jdk.attach jdk.charsets jdk.compiler jdk.crypto.ec jdk.crypto.cryptoki jdk.crypto.mscapi jdk.dynalink jdk.internal.ed jdk.editpad jdk.hotspot.agent jdk.httpserver jdk.internal.le jdk.internal.opt jdk.internal.vm.compiler.management jdk.jartool jdk.javadoc jdk.jcmd jdk.management.agent jdk.jconsole jdk.jdeps jdk.jdwp.agent jdk.jdi jdk.jfr jdk.jlink jdk.jshell jdk.jsobject jdk.jstatd jdk.localedata jdk.management.jfr jdk.naming.dns jdk.naming.ldap jdk.naming.rmi jdk.net jdk.pack jdk.rmic jdk.scripting.nashorn jdk.scripting.nashorn.shell jdk.sctp jdk.security.auth jdk.security.jgss jdk.unsupported.desktop jdk.xml.dom jdk.zipfs" OS_ARCH="x86_64" OS_NAME="Windows" SOURCE=".:git:6171c19d2c18" We can just call `StreamReader.ReadLine()` until `JAVA_VERSION="` is found. I kept the existing logic in place, with an escape hatch if it were ever needed. After a few .NET 9 previews, we could consider removing old logic entirely? This logic is considerably faster, because we no longer launch a JVM! * Before: Task ValidateJavaVersion 377ms * After: Task ValidateJavaVersion 2ms Where you can test the "Before" logic by passing `-p:_AndroidUseJavaExeVersion=true`. I assume the customer's log from before (35 calls) would be around 70ms give or take instead of 3 hours and 46 minutes? Even if this estimation was 10x worse, it would still be a huge improvement! :eyes:
I might have misread and this is 3 minutes 46 seconds |
dellis1972
approved these changes
Jan 23, 2024
grendello
added a commit
that referenced
this pull request
Jan 24, 2024
* main: LEGO: Merge pull request 8665 [Xamarin.Android.Build.Tasks] parse JDK `release` file directly (#8663)
grendello
added a commit
that referenced
this pull request
Jan 25, 2024
* main: Localized file check-in by OneLocBuild Task (#8668) [Xamarin.Android.build.Tasks] `<CheckDuplicateJavaLibraries/>` ignores `repackaged.jar` (#8664) LEGO: Merge pull request 8665 [Xamarin.Android.Build.Tasks] parse JDK `release` file directly (#8663) Bump to dotnet/installer@f91d4ca399 9.0.100-alpha.1.24070.3 (#8635) [.github] Re-enable locking issues after 30 days of inactivity (#8655) Localized file check-in by OneLocBuild Task (#8657) LEGO: Merge pull request 8656 Localized file check-in by OneLocBuild Task (#8652) Bump to xamarin/xamarin-android-tools/main@b175674 (#8644) [Xamarin.Android.Build.Tasks] remove checks for `$(UsingAndroidNETSdk)` (#8647) [Xamarin.Android.Build.Tasks] XA1039 error for Android.Support (#8629)
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes: #8369
During Android project builds, the
<ValidateJavaVersion/>MSBuild task currently shells out tojava -versionandjavac -versionto determine if we have a valid & usable JDK.Unfortunately, some customer logs have reported build times that are completely bananas:
java -versionand/orjavac -versionmust be quite slow! Reading the source for OpenJDK, it appears thatjava -versionactually starts a JVM to call a single method:That could explain the slowness?
To improve this, let's just parse the file:
With contents:
We can just call
StreamReader.ReadLine()untilJAVA_VERSION="is found.I kept the existing logic in place, with an escape hatch if it were ever needed. After a few .NET 9 previews, we could consider removing old logic entirely?
This logic is considerably faster, because we no longer launch a JVM!
Where you can test the "Before" logic by passing
-p:_AndroidUseJavaExeVersion=true.I assume the customer's log from before (35 calls) would be around 70ms give or take instead of 3 hours and 46 minutes? Even if this estimation was 10x worse, it would still be a huge improvement! 👀