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] <GetImportedLibraries/> perf improvements #2930

Merged
merged 2 commits into from
Apr 8, 2019

Conversation

jonathanpeppers
Copy link
Member

I noticed a target running on an incremental build with only a XAML
code change:

116 ms  _BuildLibraryImportsCache                  1 calls

This was using VS 2019 GA, just a blank Xamarin.Forms app.

Reviewing <GetImportedLibraries/> it had some general perf
improvements that could be done:

  • It called Directory.EnumerateFiles three times...
  • It used a bunch of LINQ where a foreach would do.

Additionally, I saw some improvements I could do at the MSBuild target
level:

<Target Name="_BuildLibraryImportsCache"
        Inputs="$(MSBuildProjectFullPath);@(_ReferencePath);@(_ReferenceDependencyPaths);$(_AndroidBuildPropertiesCache);$(_AndroidLibraryProjectImportsCache)"
        Outputs="$(_AndroidLibraryImportsCache).stamp">

This target operates on the output of _ResolveLibraryProjectImports,
but it was re-running if any assembly changed?

I think the Inputs and Outputs would more appropriately be:

Inputs="$(_AndroidLibraryProjectImportsCache)"
Outputs="$(_AndroidStampDirectory)_BuildLibraryImportsCache.stamp"

This way, if _ResolveLibraryProjectImports runs and does work, it
writes to $(_AndroidLibraryProjectImportsCache). We then only need
to run this task when that file changes.

I also updated this target so it follows our new convention for
placing stamp files in $(_AndroidStampDirectory).

Results

An example of an initial build of the Xamarin.Forms project in this
repo:

Before:
299 ms  GetImportedLibraries                       1 calls
After:
172 ms  GetImportedLibraries                       1 calls

And then an incremental build, that is a XAML change:

Before:
109 ms  _BuildLibraryImportsCache                  1 calls
After:
n/a

The task is skipped now:

_BuildLibraryImportsCache:
Skipping target "_BuildLibraryImportsCache" because all output files are up-to-date with respect to the input files.

Overall, I would say there is a ~120ms improvement on initial build.
Incremental builds with XAML changes have a ~100ms improvement.

I noticed a target running on an incremental build with only a XAML
code change:

    116 ms  _BuildLibraryImportsCache                  1 calls

This was using VS 2019 GA, just a blank Xamarin.Forms app.

Reviewing `<GetImportedLibraries/>` it had some general perf
improvements that could be done:

* It called `Directory.EnumerateFiles` three times...
* It used a bunch of LINQ where a `foreach` would do.

Additionally, I saw some improvements I could do at the MSBuild target
level:

    <Target Name="_BuildLibraryImportsCache"
            Inputs="$(MSBuildProjectFullPath);@(_ReferencePath);@(_ReferenceDependencyPaths);$(_AndroidBuildPropertiesCache);$(_AndroidLibraryProjectImportsCache)"
            Outputs="$(_AndroidLibraryImportsCache).stamp">

This target operates on the output of `_ResolveLibraryProjectImports`,
but it was re-running if *any* assembly changed?

I think the `Inputs` and `Outputs` would more appropriately be:

    Inputs="$(_AndroidLibraryProjectImportsCache)"
    Outputs="$(_AndroidStampDirectory)_BuildLibraryImportsCache.stamp"

This way, if `_ResolveLibraryProjectImports` runs and does work, it
writes to `$(_AndroidLibraryProjectImportsCache)`. We then only need
to run this task when that file changes.

I also updated this target so it follows our new convention for
placing stamp files in `$(_AndroidStampDirectory)`.

~~ Results ~~

An example of an initial build of the Xamarin.Forms project in this
repo:

    Before:
    299 ms  GetImportedLibraries                       1 calls
    After:
    172 ms  GetImportedLibraries                       1 calls

And then an incremental build, that is a XAML change:

    Before:
    109 ms  _BuildLibraryImportsCache                  1 calls
    After:
    n/a

The task is skipped now:

    _BuildLibraryImportsCache:
    Skipping target "_BuildLibraryImportsCache" because all output files are up-to-date with respect to the input files.

Overall, I would say there is a ~120ms improvement on initial build.
Incremental builds with XAML changes have a ~100ms improvement.
@jonathanpeppers
Copy link
Member Author

Build logs: getimportedlibraries.zip

@dellis1972
Copy link
Contributor

3 of the errors look like those weird ssl issues.

One is this

System.IO.FileNotFoundException: Could not find file "/Users/builder/jenkins/workspace/xamarin-android-pr-pipeline-release/xamarin-android/bin/TestRelease/TestResult-Mono.Android_TestsMultiDex.xml"

not sure it is related though,

@jonathanpeppers
Copy link
Member Author

I restarted the macOS build -- if we just had network failures would be OK, but the multi-dex tests failing to start were concerning.

@jonathanpeppers
Copy link
Member Author

I think the macOS build is OK:

image

They seem network-related, like the other PRs we have right now.

@dellis1972 dellis1972 merged commit 3f0152d into dotnet:master Apr 8, 2019
@github-actions github-actions bot locked and limited conversation to collaborators Jan 31, 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.

2 participants