Commit 0a3354b
authored
[Java.Interop.Tools.Cecil] use File.Exists instead of DirectoryGetFile (#596)
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 b81cfbb, `DirectoryAssemblyResolver:Resolv.()` is called
thousands of times during a build. Does it makes sense that 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.1 parent 27cfd45 commit 0a3354b
File tree
1 file changed
+4
-16
lines changed- src/Java.Interop.Tools.Cecil/Java.Interop.Tools.Cecil
1 file changed
+4
-16
lines changedLines changed: 4 additions & 16 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
254 | 254 | | |
255 | 255 | | |
256 | 256 | | |
257 | | - | |
258 | | - | |
| 257 | + | |
| 258 | + | |
259 | 259 | | |
260 | 260 | | |
261 | | - | |
262 | | - | |
| 261 | + | |
| 262 | + | |
263 | 263 | | |
264 | 264 | | |
265 | 265 | | |
266 | 266 | | |
267 | | - | |
268 | | - | |
269 | | - | |
270 | | - | |
271 | | - | |
272 | | - | |
273 | | - | |
274 | | - | |
275 | | - | |
276 | | - | |
277 | | - | |
278 | | - | |
279 | 267 | | |
280 | 268 | | |
0 commit comments