[Java.Interop.Tools.Cecil] avoid File.Exists() check in DirectoryAssemblyResolver
#1065
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
…ssemblyResolver`
`dotnet-trace` of a `dotnet new maui` app:
dotnet trace collect --format speedscope -- C:\src\xamarin-android\bin\Release\dotnet\dotnet.exe build -bl --no-restore bar.csproj
Shows some interesting time spent in:
179.53ms java.interop.tools.cecil!Java.Interop.Tools.Cecil.DirectoryAssemblyResolver.Load(class System.String,bool)
7.89ms system.private.corelib.il!System.IO.File.Exists(class System.String)
171.63ms java.interop.tools.cecil!Java.Interop.Tools.Cecil.DirectoryAssemblyResolver.ReadAssembly(class System.String)
24.18ms system.private.corelib.il!System.IO.File.Exists(class System.String)
For `LoadAssembly()`, the common case is that the files always exist,
and the rare case they would be missing. Instead of calling
`File.Exists()` on every assembly, we can handle
`FileNotFoundException` and `return null` appropriately.
For `ReadAssembly()` we can reorder the check for symbol files:
bool haveDebugSymbols = loadDebugSymbols &&
(File.Exists (Path.ChangeExtension (file, ".pdb")) ||
File.Exists (file + ".mdb"));
So we check for `.pdb` files first, which should more likely exist in
modern projects. We could drop `.mdb` file checks at some point, when
this code isn't shared with classic Xamarin.Android.
Testing the changes afterward:
167.57ms java.interop.tools.cecil!Java.Interop.Tools.Cecil.DirectoryAssemblyResolver.Load(class System.String,bool)
141.56ms xamarin.android.cecil!Mono.Cecil.AssemblyDefinition.ReadAssembly(class System.String,class Mono.Cecil.ReaderParameters)
There appears to be some variance in the timing, but my rough estimate
is this saves about 15-20ms on incremental builds of `dotnet new maui`
projects. Larger projects could potentially save more.
jonathanpeppers
commented
Dec 8, 2022
src/Java.Interop.Tools.Cecil/Java.Interop.Tools.Cecil/DirectoryAssemblyResolver.cs
Outdated
Show resolved
Hide resolved
jonpryor
pushed a commit
to xamarin/xamarin-android
that referenced
this pull request
Dec 12, 2022
Changes: xamarin/java.interop@3a9f770...149d70f * xamarin/java.interop@149d70fe: [generator] Refactor enum writing to use SourceWriters (xamarin/java.interop#1063) * xamarin/java.interop@c2daa9f0: [Java.Interop.Tools.Cecil] DirectoryAssemblyResolver & File.Exists() (xamarin/java.interop#1065) * xamarin/java.interop@8ab9d33a: [Java.Interop.Tools.TypeNameMappings] improve `ToJniNameFromAttributesForAndroid` (xamarin/java.interop#1064) * xamarin/java.interop@09f8da26: [JavaCallableWrappers] avoid `string.Format()` (xamarin/java.interop#1061) Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
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.
dotnet-traceof adotnet new mauiapp:Shows some interesting time spent in:
For
LoadAssembly(), the common case is that the files always exist, and the rare case they would be missing. Instead of callingFile.Exists()on every assembly, we can handleFileNotFoundExceptionandreturn nullappropriately.For
ReadAssembly()we can reorder the check for symbol files:So we check for
.pdbfiles first, which should more likely exist in modern projects. We could drop.mdbfile checks at some point, when this code isn't shared with classic Xamarin.Android.Testing the changes afterward:
There appears to be some variance in the timing, but my rough estimate is this saves about 15-20ms on incremental builds of
dotnet new mauiprojects. Larger projects could potentially save more.