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] fully incremental linker for Debug builds #3535

Merged

Conversation

jonathanpeppers
Copy link
Member

If I time an incremental build of a "Hello World" Xamarin.Forms app,
one of the last remaining areas to improve is:

340 ms  LinkAssemblies                             1 calls

In this build, I made a one-line change in a XAML file in a
NetStandard project with the optimal settings such as
$(ProduceReferenceAssembly).

On a slower PC I have with Windows defender enabled, this step can
take even longer:

767 ms  LinkAssemblies                             1 calls

Part of the reason for the time taken is the linker needs to load
every assembly, even if not every assembly is actually changed or
saved.

A Debug build (or $(AndroidLinkMode) = None) only needs to run a
single linker step:

var pipeline = new Pipeline ();
if (options.LinkNone) {
    pipeline.AppendStep (new FixAbstractMethodsStep ());
    pipeline.AppendStep (new OutputStepWithTimestamps ());
    return pipeline;
}

FixAbstractMethodsStep also only applies to "Xamarin.Android"
assemblies which would reference Mono.Android.dll. The
OutputStepWithTimestamps step is also responsible for getting all
assemblies to their final destination in
$(IntermediateOutputPath)android\assets\.

In an ideal world, in an incremental build:

  • A Foo.dll NetStandard assembly changes
    • We know it is not a "Xamarin.Android" assembly
  • The linker step loads 0 assemblies
    • We merely copy Foo.dll to android\assets\

We can achieve this with a new <LinkAssembliesNoShrink/> MSBuild
task that doesn't use the normal code path for the full linker. We can
split out FixAbstractMethodsStep so we can call it on a per-assembly
basis as needed.

Partial Builds

To achieve our ideal scenario, we need to leverage a feature of
MSBuild that enables an MSBuild target to build partially:

Building target "Foo" partially, because some output files are out of date with respect to their input files.

For example, we had a target such as:

<Target Name="_GenerateDocumentation"
    Inputs="@(_JavaFiles)"
    Outputs="@(_JavaFiles->'$(DocsDirectory)%(Filename).md')">
  <GenerateDocumentation
      SourceFiles="@(_JavaFiles)"
      DestinationFiles="@(_JavaFiles->'$(DocsDirectory)%(Filename).md')"
  />
</Target>

MSBuild can compare the Inputs and Outputs and only pass in the
@(_JavaFiles) that have changed to <GenerateDocumentation/>.

The trick, is that <LinkAssembliesNoShrink/> needs to process a
subset of the assemblies that changed, while still knowing the full
list of assemblies. An assembly could change that references another
assembly that Mono.Cecil needs to be able to load.

So we can record an @(_AllResolvedAssemblies) item group such as:

<Target Name="_LinkAssembliesNoShrinkInputs">
  <ItemGroup>
    <_AllResolvedAssemblies Include="@(ResolvedAssemblies)" />
  </ItemGroup>
</Target>

<Target Name="_LinkAssembliesNoShrink"
    DependsOnTargets="_LinkAssembliesNoShrinkInputs"
    Inputs="@(ResolvedAssemblies);$(_AndroidBuildPropertiesCache)"
    Outputs="@(ResolvedAssemblies->'$(MonoAndroidIntermediateAssemblyDir)%(Filename)%(Extension)')">
  <LinkAssembliesNoShrink
      ResolvedAssemblies="@(_AllResolvedAssemblies)"
      SourceFiles="@(ResolvedAssemblies)"
      DestinationFiles="@(ResolvedAssemblies->'$(MonoAndroidIntermediateAssemblyDir)%(Filename)%(Extension)')"
  />
...
</Target>

This way SourceFiles and DestinationFiles files have a subset of
assemblies, while ResolvedAssemblies can still have a known list of
all assemblies.

Results

Incremental builds are improved dramatically for a one-line XAML
change:

Before:
340 ms  LinkAssemblies                             1 calls
After:
  9 ms  LinkAssembliesNoShrink                     1 calls

The only I/O operation is a MonoAndroidHelper.CopyIfChanged on a
single assembly.

Other Refactoring

Since <LinkAssemblies/> is no longer used for Debug builds, we can
remove a few options that are not currently used for Release builds:

  • OptionalDestinationDirectory
  • LinkOnlyNewerThan

@jonathanpeppers jonathanpeppers force-pushed the linkassembliesnoshrink.cs branch 3 times, most recently from 56e6f73 to 797b9cc Compare August 27, 2019 16:23
@jonathanpeppers jonathanpeppers marked this pull request as ready for review August 27, 2019 16:23
@jonathanpeppers jonathanpeppers force-pushed the linkassembliesnoshrink.cs branch 2 times, most recently from 40ca005 to 287b3f4 Compare August 28, 2019 21:48
If I time an incremental build of a "Hello World" Xamarin.Forms app,
one of the last remaining areas to improve is:

    340 ms  LinkAssemblies                             1 calls

In this build, I made a one-line change in a XAML file in a
NetStandard project with the optimal settings such as
`$(ProduceReferenceAssembly)`.

On a slower PC I have with Windows defender enabled, this step can
take even longer:

    767 ms  LinkAssemblies                             1 calls

Part of the reason for the time taken is the linker needs to load
*every* assembly, even if not every assembly is actually changed or
saved.

A `Debug` build (or `$(AndroidLinkMode)` = `None`) only needs to run a
single linker step:

    var pipeline = new Pipeline ();
    if (options.LinkNone) {
        pipeline.AppendStep (new FixAbstractMethodsStep ());
        pipeline.AppendStep (new OutputStepWithTimestamps ());
        return pipeline;
    }

`FixAbstractMethodsStep` also only applies to "Xamarin.Android"
assemblies which would reference `Mono.Android.dll`. The
`OutputStepWithTimestamps` step is also responsible for getting all
assemblies to their final destination in
`$(IntermediateOutputPath)android\assets\`.

In an ideal world, in an incremental build:

* A `Foo.dll` NetStandard assembly changes
    * We know it is *not* a "Xamarin.Android" assembly
* The linker step loads 0 assemblies
    * We merely copy `Foo.dll` to `android\assets\`

We can achieve this with a new `<LinkAssembliesNoShrink/>` MSBuild
task that doesn't use the normal code path for the full linker. We can
split out `FixAbstractMethodsStep` so we can call it on a per-assembly
basis as needed.

~~ Partial Builds ~~

To achieve our ideal scenario, we need to leverage a feature of
MSBuild that enables an MSBuild target to build *partially*:

    Building target "Foo" partially, because some output files are out of date with respect to their input files.

For example, we had a target such as:

    <Target Name="_GenerateDocumentation"
        Inputs="@(_JavaFiles)"
        Outputs="@(_JavaFiles->'$(DocsDirectory)%(Filename).md')">
      <GenerateDocumentation
          SourceFiles="@(_JavaFiles)"
          DestinationFiles="@(_JavaFiles->'$(DocsDirectory)%(Filename).md')"
      />
    </Target>

MSBuild can compare the `Inputs` and `Outputs` and only pass in the
`@(_JavaFiles)` that have changed to `<GenerateDocumentation/>`.

The trick, is that `<LinkAssembliesNoShrink/>` needs to process a
subset of the assemblies that changed, while still *knowing* the full
list of assemblies. An assembly could change that references another
assembly that Mono.Cecil needs to be able to load.

So we can record an `@(_AllResolvedAssemblies)` item group such as:

    <Target Name="_LinkAssembliesNoShrinkInputs">
      <ItemGroup>
        <_AllResolvedAssemblies Include="@(ResolvedAssemblies)" />
      </ItemGroup>
    </Target>

    <Target Name="_LinkAssembliesNoShrink"
        DependsOnTargets="_LinkAssembliesNoShrinkInputs"
        Inputs="@(ResolvedAssemblies);$(_AndroidBuildPropertiesCache)"
        Outputs="@(ResolvedAssemblies->'$(MonoAndroidIntermediateAssemblyDir)%(Filename)%(Extension)')">
      <LinkAssembliesNoShrink
          ResolvedAssemblies="@(_AllResolvedAssemblies)"
          SourceFiles="@(ResolvedAssemblies)"
          DestinationFiles="@(ResolvedAssemblies->'$(MonoAndroidIntermediateAssemblyDir)%(Filename)%(Extension)')"
      />
    ...
    </Target>

This way `SourceFiles` and `DestinationFiles` files have a subset of
assemblies, while `ResolvedAssemblies` can still have a known list of
all assemblies.

~~ Results ~~

Incremental builds are improved dramatically for a one-line XAML
change:

    Before:
    340 ms  LinkAssemblies                             1 calls
    After:
      9 ms  LinkAssembliesNoShrink                     1 calls

The only I/O operation is a `MonoAndroidHelper.CopyIfChanged` on a
single assembly.

~~ Other Refactoring ~~

Since `<LinkAssemblies/>` is no longer used for `Debug` builds, we can
remove a few options that are not currently used for `Release` builds:

* `OptionalDestinationDirectory`
* `LinkOnlyNewerThan`

The `obj\Debug\linkdst` directory is also now no longer used, I
removed the `$(MonoAndroidIntermediateAssemblyTempDir)` property as
well as some tests checking it.
/// <summary>
/// This task is for Debug builds where LinkMode=None, LinkAssemblies is for Release builds
/// </summary>
public class LinkAssembliesNoShrink : Task
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't FixAbstractMethods be a more descriptive task name here?

Otherwise LGTM.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would agree, however we might add another step later on, so its probably best to keep it as a Link named task.

@radekdoulik
Copy link
Member

radekdoulik commented Sep 5, 2019

I like the new building partially feature of msbuild.

I think _GenerateJniMarshalMethods target might be another candidate for this (not for this PR of course :-) Do I understand it correctly that the target would need to have the destination files in the outputs instead of the stamp file. Or would it need more changes there?

@dellis1972 dellis1972 merged commit 5320c25 into dotnet:master Sep 5, 2019
@jonathanpeppers jonathanpeppers deleted the linkassembliesnoshrink.cs branch September 5, 2019 13:43
jonpryor pushed a commit that referenced this pull request Sep 6, 2019
…ds (#3535)

If I time an incremental build of a "Hello World" Xamarin.Forms app,
one of the last remaining areas to improve is:

    340 ms  LinkAssemblies                             1 calls

In this build, I made a one-line change in a XAML file in a
NetStandard project with the optimal settings such as
`$(ProduceReferenceAssembly)`.

On a slower PC I have with Windows defender enabled, this step can
take even longer:

    767 ms  LinkAssemblies                             1 calls

Part of the reason for the time taken is the linker needs to load
*every* assembly, even if not every assembly is actually changed or
saved.

A `Debug` build (or `$(AndroidLinkMode)` = `None`) only needs to run a
single linker step:

    var pipeline = new Pipeline ();
    if (options.LinkNone) {
        pipeline.AppendStep (new FixAbstractMethodsStep ());
        pipeline.AppendStep (new OutputStepWithTimestamps ());
        return pipeline;
    }

`FixAbstractMethodsStep` also only applies to "Xamarin.Android"
assemblies which would reference `Mono.Android.dll`. The
`OutputStepWithTimestamps` step is also responsible for getting all
assemblies to their final destination in
`$(IntermediateOutputPath)android\assets\`.

In an ideal world, in an incremental build:

* A `Foo.dll` NetStandard assembly changes
    * We know it is *not* a "Xamarin.Android" assembly
* The linker step loads 0 assemblies
    * We merely copy `Foo.dll` to `android\assets\`

We can achieve this with a new `<LinkAssembliesNoShrink/>` MSBuild
task that doesn't use the normal code path for the full linker. We can
split out `FixAbstractMethodsStep` so we can call it on a per-assembly
basis as needed.

~~ Partial Builds ~~

To achieve our ideal scenario, we need to leverage a feature of
MSBuild that enables an MSBuild target to build *partially*:

    Building target "Foo" partially, because some output files are out of date with respect to their input files.

For example, we had a target such as:

    <Target Name="_GenerateDocumentation"
        Inputs="@(_JavaFiles)"
        Outputs="@(_JavaFiles->'$(DocsDirectory)%(Filename).md')">
      <GenerateDocumentation
          SourceFiles="@(_JavaFiles)"
          DestinationFiles="@(_JavaFiles->'$(DocsDirectory)%(Filename).md')"
      />
    </Target>

MSBuild can compare the `Inputs` and `Outputs` and only pass in the
`@(_JavaFiles)` that have changed to `<GenerateDocumentation/>`.

The trick, is that `<LinkAssembliesNoShrink/>` needs to process a
subset of the assemblies that changed, while still *knowing* the full
list of assemblies. An assembly could change that references another
assembly that Mono.Cecil needs to be able to load.

So we can record an `@(_AllResolvedAssemblies)` item group such as:

    <Target Name="_LinkAssembliesNoShrinkInputs">
      <ItemGroup>
        <_AllResolvedAssemblies Include="@(ResolvedAssemblies)" />
      </ItemGroup>
    </Target>

    <Target Name="_LinkAssembliesNoShrink"
        DependsOnTargets="_LinkAssembliesNoShrinkInputs"
        Inputs="@(ResolvedAssemblies);$(_AndroidBuildPropertiesCache)"
        Outputs="@(ResolvedAssemblies->'$(MonoAndroidIntermediateAssemblyDir)%(Filename)%(Extension)')">
      <LinkAssembliesNoShrink
          ResolvedAssemblies="@(_AllResolvedAssemblies)"
          SourceFiles="@(ResolvedAssemblies)"
          DestinationFiles="@(ResolvedAssemblies->'$(MonoAndroidIntermediateAssemblyDir)%(Filename)%(Extension)')"
      />
    ...
    </Target>

This way `SourceFiles` and `DestinationFiles` files have a subset of
assemblies, while `ResolvedAssemblies` can still have a known list of
all assemblies.

~~ Results ~~

Incremental builds are improved dramatically for a one-line XAML
change:

    Before:
    340 ms  LinkAssemblies                             1 calls
    After:
      9 ms  LinkAssembliesNoShrink                     1 calls

The only I/O operation is a `MonoAndroidHelper.CopyIfChanged` on a
single assembly.

~~ Other Refactoring ~~

Since `<LinkAssemblies/>` is no longer used for `Debug` builds, we can
remove a few options that are not currently used for `Release` builds:

* `OptionalDestinationDirectory`
* `LinkOnlyNewerThan`

The `obj\Debug\linkdst` directory is also now no longer used, I
removed the `$(MonoAndroidIntermediateAssemblyTempDir)` property as
well as some tests checking it.
@github-actions github-actions bot locked and limited conversation to collaborators Jan 29, 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.

5 participants