Skip to content

Commit

Permalink
[Xamarin.Android.Build.Tasks] DTBs and _ResolveLibraryProjectImports (#…
Browse files Browse the repository at this point in the history
…3891)

I spent a little time looking at build logs of design-time builds
inside VS Windows. To trigger these, I was editing an `.axml` file and
saving it.

I found some perf issues...

The first thing I noticed was that `_ResolveLibraryProjectImports`
runs on every DTB:

    Building target "_ResolveLibraryProjectImports" completely.I spent a little time looking at build logs of design-time builds
inside VS Windows.  To trigger these, I was editing an `.axml` file
and saving it.

I found some perf issues...

The first thing I noticed was that the `_ResolveLibraryProjectImports`
target runs on every DTB:

	Building target "_ResolveLibraryProjectImports" completely.
	Input file "obj\Debug\100\designtime\build.props" is newer than output file "obj\Debug\100\stamp\_ResolveLibraryProjectImports.stamp".

Then I looked at how `build.props` was actually changing between
different targets such as:

  * `UpdateGeneratedFiles`: the general DTB target
  * `SetupDependenciesForDesigner`: the Android designer runs this
    target

Comparing the files, I saw this difference:

	- AndroidSdkPath = C:\Program Files (x86)\Android\android-sdk
	+ AndroidSdkPath = C:\Program Files (x86)\Android\android-sdk\
	- AndroidNdkPath = C:\Program Files (x86)\Android\android-sdk\ndk-bundle
	+ AndroidNdkPath = C:\Program Files (x86)\Android\android-sdk\ndk-bundle\
	- JavaSdkPath = C:\Program Files\Android\Jdk\microsoft_dist_openjdk_1.8.0.25
	+ JavaSdkPath = C:\Program Files\Android\Jdk\microsoft_dist_openjdk_1.8.0.25\

So in some cases the `_ResolveMonoAndroidSdks` target is running to
normalize the paths and other times not!

The `_CreatePropertiesCache` target should depend on the
`_ResolveMonoAndroidSdks` target to fix this issue.

After that I found the *next* issue:

	- DesignTimeBuild = True
	+ DesignTimeBuild = true

What??? It appears that the casing differs depending on which part of
the IDE is triggering the build.  I suspect the designer is using one
casing while VS proper uses another.

I think we should call `ToLowerInvariant()` on every line in the
`build.props` file to work around MSBuild's case insensitivity.

This change saves ~350ms on every design-time build.  However, if
`build.props` changes all the time it may do even better, since
`build.props` triggers almost all MSBuild targets in Xamarin.Android.
    Input file "obj\Debug\100\designtime\build.props" is newer than output file "obj\Debug\100\stamp\_ResolveLibraryProjectImports.stamp".

Then I looked at how `build.props` was actually changing between
different targets such as:

* `UpdateGeneratedFiles` - the general DTB target
* `SetupDependenciesForDesigner` - the Android designer runs this
  target

Comparing the files, I saw this difference:

    - AndroidSdkPath = C:\Program Files (x86)\Android\android-sdk
    + AndroidSdkPath = C:\Program Files (x86)\Android\android-sdk\
    - AndroidNdkPath = C:\Program Files (x86)\Android\android-sdk\ndk-bundle
    + AndroidNdkPath = C:\Program Files (x86)\Android\android-sdk\ndk-bundle\
    - JavaSdkPath = C:\Program Files\Android\Jdk\microsoft_dist_openjdk_1.8.0.25
    + JavaSdkPath = C:\Program Files\Android\Jdk\microsoft_dist_openjdk_1.8.0.25\

So in some cases `_ResolveMonoAndroidSdks` is running to normalize the
paths and other times not... `_CreatePropertiesCache` should depend on
`_ResolveMonoAndroidSdks` to fix this issue.

After that I found the *next* issue:

    - DesignTimeBuild = True
    + DesignTimeBuild = true

What??? It appears that the casing differs depending on which part of
the IDE is triggering the build. I suspect the designer is using one
casing while VS proper uses another.

I think we should call `ToLowerInvariant()` on every line in the
`build.props` file to work around MSBuild's case insensitivity.

This change saves ~350ms on every design-time build. However, if
`build.props` changes all the time it may do even better, since
`build.props` triggers almost all MSBuild targets in Xamarin.Android.
  • Loading branch information
jonathanpeppers authored and jonpryor committed Nov 12, 2019
1 parent 6e55c22 commit 58b47b7
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -950,6 +950,22 @@ public void DeterministicBuilds ([Values (true, false)] bool deterministic)
}
}

[Test]
public void DesignTimeBuild ()
{
var proj = new XamarinAndroidApplicationProject ();
using (var b = CreateApkBuilder (Path.Combine ("temp", $"{nameof (IncrementalBuildTest)}{TestName}"))) {
Assert.IsTrue (b.DesignTimeBuild (proj), "first dtb should have succeeded.");
// DesignTimeBuild=true lowercased
var parameters = new [] { "DesignTimeBuild=true" };
Assert.IsTrue (b.RunTarget (proj, "Compile", doNotCleanupOnUpdate: true, parameters: parameters), "second dtb should have succeeded.");
var target = "_ResolveLibraryProjectImports";
Assert.IsTrue (b.Output.IsTargetSkipped (target), $"`{target}` should have been skipped.");
Assert.IsTrue (b.RunTarget (proj, "UpdateGeneratedFiles", doNotCleanupOnUpdate: true, parameters: parameters), "UpdateGeneratedFiles should have succeeded.");
Assert.IsTrue (b.Output.IsTargetSkipped (target), $"`{target}` should have been skipped.");
}
}

[Test]
public void ChangePackageNamingPolicy ()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1172,7 +1172,7 @@ because xbuild doesn't support framework reference assemblies.
<_AndroidManagedResourceDesignerFile>$(_AndroidIntermediateDesignTimeBuildDirectory)$(_AndroidResourceDesigner)</_AndroidManagedResourceDesignerFile>
</PropertyGroup>

<Target Name="_CreatePropertiesCache" DependsOnTargets="_SetupDesignTimeBuildForBuild;_SetLatestTargetFrameworkVersion">
<Target Name="_CreatePropertiesCache" DependsOnTargets="_SetupDesignTimeBuildForBuild;_SetLatestTargetFrameworkVersion;_ResolveMonoAndroidSdks">
<ItemGroup>
<!--- List of items we want to trigger a build if changed -->
<_PropertyCacheItems Include="BundleAssemblies=$(BundleAssemblies)" />
Expand Down Expand Up @@ -1206,7 +1206,7 @@ because xbuild doesn't support framework reference assemblies.
<MakeDir Directories="$(_AndroidStampDirectory)" Condition="!Exists('$(_AndroidStampDirectory)')" />
<WriteLinesToFile
File="$(_AndroidBuildPropertiesCache)"
Lines="@(_PropertyCacheItems)"
Lines="@(_PropertyCacheItems->ToLowerInvariant())"
Overwrite="true"
WriteOnlyWhenDifferent="true"
/>
Expand Down

0 comments on commit 58b47b7

Please sign in to comment.