From 31b6a4e3c25c8a2bef35f2ac7387e159b4d9fb16 Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Fri, 6 Mar 2020 15:52:14 -0600 Subject: [PATCH] [Java.Interop.Tools.Cecil] use File.Exists instead of DirectoryGetFile MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Context: https://www.jetbrains.com/profiler/ I have been playing with dotTrace, and it displayed the following as the no. 4 top "hot path" during a build of the Xamarin.Forms integration project: 0.08% DirectoryGetFile • 376/0 ms • Java.Interop.Tools.Cecil.DirectoryAssemblyResolver.DirectoryGetFile(String, String) 0.08% SearchDirectory • Java.Interop.Tools.Cecil.DirectoryAssemblyResolver.SearchDirectory(String, String) 0.08% Resolve • Java.Interop.Tools.Cecil.DirectoryAssemblyResolver.Resolve(AssemblyNameReference, ReaderParameters) 0.05% RunTask • 262 of 376 ms • Xamarin.Android.Tasks.LinkAssembliesNoShrink.RunTask 0.02% Resolve • 114 of 376 ms • Java.Interop.Tools.Cecil.DirectoryAssemblyResolver.Resolve(AssemblyNameReference) As seen in b81cfbb9, `DirectoryAssemblyResolver:Resolve` is called thousands of times during a build. It makes sense this would show up in a profiler? Looking at the code: static string DirectoryGetFile (string directory, string file) { if (!Directory.Exists (directory)) return ""; var files = Directory.GetFiles (directory, file); if (files != null && files.Length > 0) return files [0]; return ""; } It seems this will always allocate a `string[]`, and it's calling `Directory.GetFiles` using the filename as a wildcard. I thought a `File.Exists` check would be better? Comparing the two methods with Benchmark.NET: https://github.com/jonathanpeppers/Benchmarks/blob/d36d72eaf7fc82f20137c47ff768ffc38392f938/Benchmarks/FilesBenchmarks.cs BenchmarkDotNet=v0.12.0, OS=Windows 10.0.18362 Intel Core i9-9900K CPU 3.60GHz (Coffee Lake), 1 CPU, 16 logical and 8 physical cores [Host] : .NET Framework 4.8 (4.8.4121.0), X86 LegacyJIT | Method | Mean | Error | StdDev | Gen 0 | Gen 1 | Gen 2 | Allocated | |-------- |---------:|---------:|---------:|-------:|------:|------:|----------:| | File.Exists | 20.88 us | 0.393 us | 0.349 us | 0.0305 | - | - | 297 B | | DirectoryGetFile | 56.28 us | 0.308 us | 0.288 us | 0.2441 | - | - | 1282 B | It seems that `File.Exists` will be ~60% better. After making that change, I saw an improvement when building the Xamarin.Forms integration project on Windows: Before: 711 ms LinkAssembliesNoShrink 1 calls After: 426 ms LinkAssembliesNoShrink 1 calls It seems like this will save ~290ms on the initial build. This target builds partially on incremental builds, so we would see a smaller improvement there. --- .../DirectoryAssemblyResolver.cs | 20 ++++--------------- 1 file changed, 4 insertions(+), 16 deletions(-) diff --git a/src/Java.Interop.Tools.Cecil/Java.Interop.Tools.Cecil/DirectoryAssemblyResolver.cs b/src/Java.Interop.Tools.Cecil/Java.Interop.Tools.Cecil/DirectoryAssemblyResolver.cs index 921e9bef9..b4dcd1d85 100644 --- a/src/Java.Interop.Tools.Cecil/Java.Interop.Tools.Cecil/DirectoryAssemblyResolver.cs +++ b/src/Java.Interop.Tools.Cecil/Java.Interop.Tools.Cecil/DirectoryAssemblyResolver.cs @@ -254,27 +254,15 @@ string SearchDirectory (string name, string directory) if (Path.IsPathRooted (name) && File.Exists (name)) return name; - var file = DirectoryGetFile (directory, name + ".dll"); - if (file.Length > 0) + var file = Path.Combine (directory, name + ".dll"); + if (File.Exists (file)) return file; - file = DirectoryGetFile (directory, name + ".exe"); - if (file.Length > 0) + file = Path.Combine (directory, name + ".exe"); + if (File.Exists (file)) return file; return null; } - - static string DirectoryGetFile (string directory, string file) - { - if (!Directory.Exists (directory)) - return ""; - - var files = Directory.GetFiles (directory, file); - if (files != null && files.Length > 0) - return files [0]; - - return ""; - } } }