Skip to content
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

[Xamarin.Android.Build.Tasks] avoid File.Exists() check #7621

Merged

Conversation

jonathanpeppers
Copy link
Member

dotnet-trace output 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 an interesting case of File.Exists():

49.88ms xamarin.android.build.tasks!Xamarin.Android.Tasks.FilterAssemblies.RunTask()
 8.09ms System.Private.CoreLib.il!System.IO.File.Exists(class System.String)

The common case is the files always exist. The rare case appears to be 783ac9e, which would be when something like a <ProjectReference/> doesn't exist.

Instead of calling File.Exists() on every .NET assembly (likely done multiple times in a build), we can handle FileNotFoundException and take appropriate action.

This will save ~8ms on every build, and we should get the exact same behavior as before. Potentially larger projects could even save more.

`dotnet-trace` output 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 an interesting case of `File.Exists()`:

    49.88ms xamarin.android.build.tasks!Xamarin.Android.Tasks.FilterAssemblies.RunTask()
     8.09ms System.Private.CoreLib.il!System.IO.File.Exists(class System.String)

The common case is the files always exist. The rare case appears to be
783ac9e, which would be when something like a `<ProjectReference/>`
doesn't exist.

Instead of calling `File.Exists()` on every .NET assembly (likely done
multiple times in a build), we can handle `FileNotFoundException` and
take appropriate action.

This will save ~8ms on *every* build, and we should get the exact same
behavior as before. Potentially larger projects could even save more.
@dellis1972
Copy link
Contributor

@jonathanpeppers I suspect the DeduplicateAssemblies method in ProcessAssemblies can use the same treatment. It would also help use remove some Linq usage

void DeduplicateAssemblies (List<ITaskItem> output, Dictionary<string, ITaskItem> symbols)

@jonathanpeppers
Copy link
Member Author

Yeah <ProcessAssemblies/> might be worth looking at later. It has a faster code path when there is 1 RID, which should be the case for Debug builds:

// We only need to "dedup" assemblies when there is more than one RID
if (RuntimeIdentifiers.Length > 1) {
Log.LogDebugMessage ("Deduplicating assemblies per RuntimeIdentifier");
DeduplicateAssemblies (output, symbols);
} else {
Log.LogDebugMessage ("Found a single RuntimeIdentifier");
SetMetadataForAssemblies (output, symbols);
}

So that might be why I didn't see this one.

@jonathanpeppers jonathanpeppers marked this pull request as draft December 8, 2022 20:53
@jonathanpeppers
Copy link
Member Author

Ok, I need to handle DirectoryNotFoundException as well.

@jonathanpeppers jonathanpeppers marked this pull request as ready for review December 9, 2022 14:15
@jonathanpeppers jonathanpeppers merged commit f270fbd into xamarin:main Dec 9, 2022
@jonathanpeppers jonathanpeppers deleted the filterassemblies-fileexists branch December 9, 2022 19:59
grendello added a commit to grendello/xamarin-android that referenced this pull request Jan 4, 2023
* main:
  [Xamarin.Android.Build.Tasks] use %(TrimmerRootAssembly.RootMode) All (xamarin#7651)
  Bump to dotnet/installer@47a747f 8.0.100-alpha.1.22616.7 (xamarin#7647)
  Bump to dotnet/installer@167a4ed 8.0.100-alpha.1.22611.1 (xamarin#7630)
  Bump to xamarin/java.interop@f8d77fa (xamarin#7638)
  [ci] Fix Designer test paths (xamarin#7635)
  [Xamarin.Android.Build.Tasks] perf improvements for LlvmIrGenerator (xamarin#7626)
  [Xamarin.Android.Build.Tasks] AutoImport `*.webp` files (xamarin#7601)
  Localized file check-in by OneLocBuild Task (xamarin#7632)
  Bump $(ProductVersion) to 13.2.99
  Bump to xamarin/monodroid@c0c933b7 (xamarin#7633)
  [Xamarin.Android.Build.Tasks] Fix aapt_rules.txt corruption (xamarin#7587)
  [Xamarin.Android.Build.Tasks] Add XA1031 error (xamarin#7448)
  Bump to xamarin/java.interop@149d70f (xamarin#7625)
  [CODEOWNERS] More updates to CODEOWNERS (xamarin#7628)
  [Xamarin.Android.Build.Tasks] avoid `File.Exists()` check (xamarin#7621)
@github-actions github-actions bot locked and limited conversation to collaborators Jan 23, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants