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] improve incremental design-time builds #6406

Merged

Conversation

jonathanpeppers
Copy link
Member

@jonathanpeppers jonathanpeppers commented Oct 19, 2021

I was testing .NET MAUI projects inside Visual Studio, and noticed on
solution load two sets of builds run for each $(TargetFramework):

  1. CollectPackageReferences;CollectFrameworkReferences;CollectUpToDateCheckBuiltDesignTime;CollectPackageDownloads;GenerateSupportedTargetFrameworkAlias;CollectAnalyzersDesignTime;CollectUpToDateCheckInputDesignTime;CollectUpToDateCheckOutputDesignTime;CollectSuggestedWorkloads;CollectCentralPackageVersions;CompileDesignTime;CollectResolvedCompilationReferencesDesignTime
  2. ResolveFrameworkReferencesDesignTime;ResolveProjectReferencesDesignTime2;CollectResolvedSDKReferencesDesignTime;ResolveComReferencesDesignTime;ResolveAssemblyReferencesDesignTime;ResolvePackageDependenciesDesignTime;CollectSuggestedWorkloads
  1. net6.0-maccatalyst 0.344s
  2. net6.0-ios 0.353s
  3. net6.0-android 1.466s
  4. net6.0-maccatalyst 0.063s
  5. net6.0-ios 0.059s
  6. net6.0-android 1.424s

image

Android is the worst in both cases, due to:

GenerateResourceDesigner 737ms
GenerateResourceDesigner 766ms

I'm not sure how I got in this state, but the
_ManagedUpdateAndroidResgen MSBuild target never skips for me:

Building target "_ManagedUpdateAndroidResgen" completely.
Input file "C:\Program Files\dotnet\sdk-manifests\6.0.100\microsoft.net.sdk.maui\WorkloadManifest.targets" is newer than output file "obj\Debug\net6.0-android\designtime\Resource.designer.cs".

Reviewing the <GenerateResourceDesigner/> MSBuild task, there is a
case it doesn't update the timestamp on the Resource.designer.cs
output file. I added a <Touch/> call to solve this issue:

<Touch Condition="Exists ('$(_AndroidManagedResourceDesignerFile)')" Files="$(_AndroidManagedResourceDesignerFile)" />

This is a nice solution, as it is localized to the MSBuild target where the
Inputs and Outputs are.

While I was at it, I switched from using a new StreamWriter() to
MemoryStreamPool. MemoryStreamPool usage saw a modest improvement:

Before:
GenerateResourceDesigner 491ms
After:
GenerateResourceDesigner 480ms

Overall these changes should improve all design-time builds by ~11ms,
and some incremental design-time builds will properly skip: saving up
to ~766ms.

@jonpryor
Copy link
Member

Alternate solution idea: instead of updating <GenerateResourceDesigner/> to "touch" the file, thus implicitly requiring that that the task "know about" it's calling target, why don't we instead update the _ManagedUpdateAndroidResgen target?

<Touch Files="$(_AndroidManagedResourceDesignerFile)" />

This keeps the logic within the target.

I was testing .NET MAUI projects inside Visual Studio, and noticed on
solution load two sets of builds run for each `$(TargetFramework)`:

1. `CollectPackageReferences;CollectFrameworkReferences;CollectUpToDateCheckBuiltDesignTime;CollectPackageDownloads;GenerateSupportedTargetFrameworkAlias;CollectAnalyzersDesignTime;CollectUpToDateCheckInputDesignTime;CollectUpToDateCheckOutputDesignTime;CollectSuggestedWorkloads;CollectCentralPackageVersions;CompileDesignTime;CollectResolvedCompilationReferencesDesignTime`
2. `ResolveFrameworkReferencesDesignTime;ResolveProjectReferencesDesignTime2;CollectResolvedSDKReferencesDesignTime;ResolveComReferencesDesignTime;ResolveAssemblyReferencesDesignTime;ResolvePackageDependenciesDesignTime;CollectSuggestedWorkloads`

1) net6.0-maccatalyst 0.344s
1) net6.0-ios         0.353s
1) net6.0-android     1.466s
2) net6.0-maccatalyst 0.063s
2) net6.0-ios         0.059s
2) net6.0-android     1.424s

Android is the worst in both cases, due to:

    GenerateResourceDesigner 737ms
    GenerateResourceDesigner 766ms

I'm not sure how I got in this state, but the
`_ManagedUpdateAndroidResgen` MSBuild target *never* skips for me:

    Building target "_ManagedUpdateAndroidResgen" completely.
    Input file "C:\Program Files\dotnet\sdk-manifests\6.0.100\microsoft.net.sdk.maui\WorkloadManifest.targets" is newer than output file "obj\Debug\net6.0-android\designtime\Resource.designer.cs".

Reviewing the `<GenerateResourceDesigner/>` MSBuild task, there is a
case it doesn't update the timestamp on the `Resource.designer.cs`
output file. It is missing the pattern we have in other tasks:

    if (Files.CopyIfStreamChanged (o.BaseStream, file)) {
        Log.LogDebugMessage ($"Writing to: {file}");
    } else {
        // NOTE: We still need to update the timestamp on this file, or the target would run again
        File.SetLastWriteTimeUtc (file, DateTime.UtcNow);
    }

While I was at it, I switched from using a `new StreamWriter()` to
`MemoryStreamPool`. `MemoryStreamPool` usage saw a modest improvement:

    Before:
    GenerateResourceDesigner 491ms
    After:
    GenerateResourceDesigner 480ms

Overall these changes should improve all design-time builds by ~11ms,
and some incremental design-time builds will properly skip: saving up
to ~766ms.
@jonathanpeppers
Copy link
Member Author

I switched to using <Touch/>:

image

I looked through the other code that uses CopyIfChanged. It appears this issue is only in the _ManagedUpdateAndroidResgen target.

@dellis1972
Copy link
Contributor

This looks good. the commit message needs updating to reflect the new changed (i.e using Touch).

@jonathanpeppers
Copy link
Member Author

I updated the PR description, we can copy paste that when we merge.

@jonathanpeppers jonathanpeppers merged commit eea09f9 into xamarin:main Oct 20, 2021
@jonathanpeppers jonathanpeppers deleted the dotnet-designtime-build-fixes branch October 20, 2021 20:41
@github-actions github-actions bot locked and limited conversation to collaborators Jan 24, 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.

None yet

4 participants