Skip to content

Commit

Permalink
[Xamarin.Android.Build.Tasks] more ConvertResourcesCases improvements (
Browse files Browse the repository at this point in the history
…#2328)

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?
  • Loading branch information
jonathanpeppers authored and dellis1972 committed Oct 23, 2018
1 parent fa57aa8 commit f7769ef
Showing 1 changed file with 20 additions and 13 deletions.
33 changes: 20 additions & 13 deletions src/Xamarin.Android.Build.Tasks/Utilities/AndroidResource.cs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ public static bool UpdateXmlResource (string res, string filename, Dictionary<st

static readonly XNamespace android = "http://schemas.android.com/apk/res/android";
static readonly XNamespace res_auto = "http://schemas.android.com/apk/res-auto";
static readonly Regex r = new Regex (@"^@\+?(?<package>[^:]+:)?(anim|color|drawable|layout|menu)/(?<file>.*)$");
static readonly Regex r = new Regex (@"^@\+?(?<package>[^:]+:)?(anim|color|drawable|layout|menu)/(?<file>.*)$", RegexOptions.Compiled);
static readonly string[] fixResourcesAliasPaths = {
"/resources/item",
"/resources/integer-array/item",
Expand Down Expand Up @@ -104,38 +104,45 @@ 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
// form: @x/y. Checking it here saves some time
// 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;
Expand Down

0 comments on commit f7769ef

Please sign in to comment.