From ac6a6021551eaba55f575b5800b591c36fca63e7 Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Mon, 22 Oct 2018 11:44:04 -0500 Subject: [PATCH] [Xamarin.Android.Build.Tasks] more ConvertResourcesCases improvements Context: http://www.getcodetrack.com/ I used a "freeware" .NET profiler to see what it would turn up. Using the tool, I profiled the following .NET process: .\bin\Debug\bin\xabuild.exe .\tests\Xamarin.Forms-Performance-Integration\Droid\Xamarin.Forms.Performance.Integration.Droid.csproj /v:quiet This seemed to have a lot of useful information, and I quickly noticed: Monodroid.AndroidResource.UpdateXmlResource -> 3683 calls -> 2.13s .. Monodroid.AndroidResource.ResourceNeedsToBeLowerCased -> 2384 calls - 319.53ms This is a known codepath, the `ConvertResourcesCases` MSBuild task, that we know takes a bit of time. Looking through the call stack, I saw two points we could make easy fixes for: - A `Regex` in `AndroidResource` was not using `RegexOptions.Compiled` - There was a string append `dirPrefix + "*"` in a LINQ expression that could be done up front. - There was a bit of LINQ usage such as: if (Directory.EnumerateDirectories (resourceBasePath, dirPrefix + "*").Any (dir => Directory.EnumerateFiles (dir, fileNamePattern).Any ())) So I added `RegexOptions.Compiled` and converted the LINQ expressions to `foreach` loops. I could see a noticeable change in these methods which moved further down the list of methods, when sorted by time duration. But since the time differences *in the profiler* likely aren't going to reflect the real world, I did a before/after comparision with the Xamarin.Forms.ControlGallery project. Project here: https://github.com/jonathanpeppers/Xamarin.Forms/tree/msbuild-timing/Xamarin.Forms.ControlGallery.Android Script here: https://github.com/jonathanpeppers/msbuild-timing/blob/master/xamarin.forms.ps1 Before: 7224 ms ConvertResourcesCases 6 calls After: 7034 ms ConvertResourcesCases 6 calls So about a 200ms improvement on Windows, for adjusting a bit of code. I suspect the improvement won't be as good on MacOS, since I don't believe `RegexOptions.Compiled` is implemented in Mono. (It is safely ignored) Other changes: - Code such as `(value ?? String.Empty).Trim ();` - Simplified to `value?.Trim ();` - Removed unused `filePath` variable Future changes: It may be worth auditing all regexes and adding `RegexOptions.Compiled` if they appear to be called a lot? --- .../Utilities/AndroidResource.cs | 33 +++++++++++-------- 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/AndroidResource.cs b/src/Xamarin.Android.Build.Tasks/Utilities/AndroidResource.cs index e8bc4da1c11..d672dc3889b 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/AndroidResource.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/AndroidResource.cs @@ -37,7 +37,7 @@ public static bool UpdateXmlResource (string res, string filename, Dictionary[^:]+:)?(anim|color|drawable|layout|menu)/(?.*)$"); + static readonly Regex r = new Regex (@"^@\+?(?[^:]+:)?(anim|color|drawable|layout|menu)/(?.*)$", RegexOptions.Compiled); static readonly string[] fixResourcesAliasPaths = { "/resources/item", "/resources/integer-array/item", @@ -104,13 +104,13 @@ static bool ResourceNeedsToBeLowerCased (string value, string resourceBasePath, // Might be a bit of an overkill, but the data comes (indirectly) from the user since it's the // path to the msbuild's intermediate output directory and that location can be changed by the // user. It's better to be safe than sorry. - resourceBasePath = (resourceBasePath ?? String.Empty).Trim (); - if (String.IsNullOrEmpty (resourceBasePath)) + resourceBasePath = resourceBasePath?.Trim (); + if (string.IsNullOrEmpty (resourceBasePath)) return true; // Avoid resource names that are all whitespace - value = (value ?? String.Empty).Trim (); - if (String.IsNullOrEmpty (value)) + value = value?.Trim (); + if (string.IsNullOrEmpty (value)) return false; // let's save some time if (value.Length < 4 || value [0] != '@') // 4 is the minimum length since we need a string // that is at least of the following @@ -118,24 +118,31 @@ static bool ResourceNeedsToBeLowerCased (string value, string resourceBasePath, // below. return true; - string filePath = null; int slash = value.IndexOf ('/'); int colon = value.IndexOf (':'); if (colon == -1) colon = 0; // Determine the the potential definition file's path based on the resource type. - string dirPrefix = value.Substring (colon + 1, slash - colon - 1).ToLowerInvariant (); + string dirPattern = value.Substring (colon + 1, slash - colon - 1).ToLowerInvariant () + "*"; string fileNamePattern = value.Substring (slash + 1).ToLowerInvariant () + ".*"; - if (Directory.EnumerateDirectories (resourceBasePath, dirPrefix + "*").Any (dir => Directory.EnumerateFiles (dir, fileNamePattern).Any ())) - return true; + foreach (var dir in Directory.EnumerateDirectories (resourceBasePath, dirPattern)) { + foreach (var file in Directory.EnumerateFiles (dir, fileNamePattern)) { + return true; + } + } // check additional directories if we have them incase the resource is in a library project - if (additionalDirectories != null) - foreach (var additionalDirectory in additionalDirectories) - if (Directory.EnumerateDirectories (additionalDirectory, dirPrefix + "*").Any (dir => Directory.EnumerateFiles (dir, fileNamePattern).Any ())) - return true; + if (additionalDirectories != null) { + foreach (var additionalDirectory in additionalDirectories) { + foreach (var dir in Directory.EnumerateDirectories (additionalDirectory, dirPattern)) { + foreach (var file in Directory.EnumerateFiles (dir, fileNamePattern)) { + return true; + } + } + } + } // No need to change the reference case. return false;