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

[resizetizer] fix a build performance issue #24453

Merged

Conversation

jonathanpeppers
Copy link
Member

@jonathanpeppers jonathanpeppers commented Aug 26, 2024

I was testing a .NET MAUI project template in Visual Studio on a Windows DevBox, and I noticed a target taking a lot of time:

ResizetizeImages 1.302s
Skipping target "ResizetizeImages" because all output files are up-to-date with respect to the input files.

This is happening on incremental builds with no changes. This is even worse for Android, because it runs twice: one for build and one for deploy.

The ResizetizeImages MSBuild target is skipped, why is it slow?!?

To "debug" this, I added this ugly, one-liner between every <ItemGroup> in the target:

<ItemGroup>
  </ItemGroup><PropertyGroup><_Time>$([System.DateTime]::Now.ToString('THH:mm:ss.fffffffZ'))</_Time></PropertyGroup><ItemGroup>
  <_MauiImageToProcess Include="@(MauiImage)" Exclude="$(DefaultItemExcludes)" />
  </ItemGroup><PropertyGroup><_Time>$([System.DateTime]::Now.ToString('THH:mm:ss.fffffffZ'))</_Time></PropertyGroup><ItemGroup>
  <!-- more item groups -->
  </ItemGroup><PropertyGroup><_Time>$([System.DateTime]::Now.ToString('THH:mm:ss.fffffffZ'))</_Time></PropertyGroup><ItemGroup>
</ItemGroup>

(I found the /profileevaluation switch didn't help me here)

This led me to find the problematic line, is this one:

<_MauiImageToProcess Include="@(MauiImage)" Exclude="$(DefaultItemExcludes)" />

image

Where $(DefaultItemExcludes) is:

DefaultItemExcludes = ;bin\Debug\/**;obj\Debug\/**;bin\/**;obj\/**;**/*.user;**/*.*proj;**/*.sln;**/*.vssscc;**/.DS_Store

This was introduced in a92fd1d to fix .DS_Store files in the folder:

<MauiImage Include="Resources\Images\*" />

(This is in the .NET MAUI project template).

Here is additinoal timing information, if I try some powershell commands like:

> $files = ls -r -n .\obj\
> $files.Length
5356
> (Measure-Command { $files = ls -r -n .\obj\ }).TotalMilliseconds
715.7382

What is happening is:

  • MSBuild expands these 9 wildcards to a lot of files

  • MSBuild now has to filter out the files that are in @(MauiImage)

If I simply reduce this to:

<_MauiImageToProcess Include="@(MauiImage)" Exclude="**/.DS_Store" />

It still takes 177ms after this change, because MSBuild has to do a recursive file listing of the current project.

So, we can do better. We can use Condition instead:

<_MauiImageToProcess Include="@(MauiImage)" Condition=" '%(FileName)%(Extension)' != '.DS_Store' " />

After these changes, the target now takes:

ResizetizeImages 6ms

I can't think of what this would break, as I don't think any of these files are in Resources\Images:

DefaultItemExcludes = ;bin\Debug\/**;obj\Debug\/**;bin\/**;obj\/**;**/*.user;**/*.*proj;**/*.sln;**/*.vssscc;**/.DS_Store

The only problem I can think of would be if someone sets $(DefaultItemExcludes) to a custom value, and I think saving the build time is worth it.

I was testing a .NET MAUI project template in Visual Studio on a
Windows DevBox, and I noticed a target taking *a lot* of time:

    ResizetizeImages 1.302s
    Skipping target "ResizetizeImages" because all output files are up-to-date with respect to the input files.

This is happening on incremental builds with no changes. This is even
worse for Android, because it runs twice: one for build and one for
deploy.

The `ResizetizeImages` MSBuild target is *skipped*, why is it slow?!?

To "debug" this, I added this ugly, one-liner between every
`<ItemGroup>` in the target:

    <ItemGroup>
      </ItemGroup><PropertyGroup><_Time>$([System.DateTime]::Now.ToString('THH:mm:ss.fffffffZ'))</_Time></PropertyGroup><ItemGroup>
      <_MauiImageToProcess Include="@(MauiImage)" Exclude="$(DefaultItemExcludes)" />
      </ItemGroup><PropertyGroup><_Time>$([System.DateTime]::Now.ToString('THH:mm:ss.fffffffZ'))</_Time></PropertyGroup><ItemGroup>
      <!-- more item groups -->
      </ItemGroup><PropertyGroup><_Time>$([System.DateTime]::Now.ToString('THH:mm:ss.fffffffZ'))</_Time></PropertyGroup><ItemGroup>
    </ItemGroup>

(I found the `/profileevaluation` switch didn't help me here)

This led me to find the problematic line, is this one:

    <_MauiImageToProcess Include="@(MauiImage)" Exclude="$(DefaultItemExcludes)" />

Where `$(DefaultItemExcludes)` is:

    DefaultItemExcludes = ;bin\Debug\/**;obj\Debug\/**;bin\/**;obj\/**;**/*.user;**/*.*proj;**/*.sln;**/*.vssscc;**/.DS_Store

This was introduced in a92fd1d to fix `.DS_Store` files in the folder:

    <MauiImage Include="Resources\Images\*" />

(This is in the .NET MAUI project template).

What is happening is:

* MSBuild expands these 9 wildcards to a lot of files

* MSBuild now has to filter out the files that are in `@(MauiImage)`

If I simply reduce this to:

    <_MauiImageToProcess Include="@(MauiImage)" Exclude="**/.DS_Store" />

It still takes 177ms after this change, because MSBuild has to
do a recursive file listing of the current project.

So, we can do better. We can use `Condition` instead:

    <_MauiImageToProcess Include="@(MauiImage)" Condition=" '%(FileName)%(Extension)' != '.DS_Store' " />

After these changes, the target now takes:

    ResizetizeImages 6ms

I can't think of what this would break, as I don't think any of these
files are in `Resources\Images`:

    DefaultItemExcludes = ;bin\Debug\/**;obj\Debug\/**;bin\/**;obj\/**;**/*.user;**/*.*proj;**/*.sln;**/*.vssscc;**/.DS_Store

The only problem I can think of would be if someone sets
`$(DefaultItemExcludes)` to a custom value, and I think saving the
build time is worth it.
@jonathanpeppers jonathanpeppers requested a review from a team as a code owner August 26, 2024 22:22
@jonathanpeppers jonathanpeppers added the area-single-project Splash Screen, Multi-Targeting, MauiFont, MauiImage, MauiAsset, Resizetizer label Aug 26, 2024
@MartinLichtblau
Copy link

Damn it! Have been looking into this for far too long and just now I wanted to give up!
Really annoyed me, seeing Android Debug Run being stuck at "Skipping target ResizetizeImages", taking ~7sec of 14sec total build time.
Thank you for finding the culprit!
Hope this will be released soon! Cause, I can't see how to implement a workaround in our app.

@jonathanpeppers
Copy link
Member Author

Hope this will be released soon! Cause, I can't see how to implement a workaround in our app.

@MartinLichtblau the only idea I have as a workaround might be to just clear $(DefaultItemExcludes) like:

<DefaultItemExcludes></DefaultItemExcludes>

But that might cause other issues.

@mattleibow
Copy link
Member

How does the SDK do it all over the show and we can't do it this one time? How are they not running a build for 30s with all these usages of the property?

https://github.com/search?q=repo%3Adotnet%2Fsdk%20DefaultItemExcludes&type=code

@jonathanpeppers
Copy link
Member Author

@mattleibow there could be a regression in MSBuild, but I think there is a big difference if it happens during evaluation or inside a target.

@jonathanpeppers
Copy link
Member Author

This one timed out:

Controls (v17.2) TabbedPage,TableView,TimePicker,TitleView,ToolbarItem

But it says I can merge, so going for it. This change shouldn't affect UITests.

@jonathanpeppers jonathanpeppers merged commit 1b00140 into dotnet:main Aug 27, 2024
96 of 97 checks passed
@jonathanpeppers jonathanpeppers deleted the DefaultItemExcludesOhNo branch August 27, 2024 17:12
@samhouts samhouts added fixed-in-net9.0-nightly This may be available in a nightly release! fixed-in-net8.0-nightly This may be available in a nightly release! labels Sep 5, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Nov 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-single-project Splash Screen, Multi-Targeting, MauiFont, MauiImage, MauiAsset, Resizetizer delighter-sc fixed-in-net8.0-nightly This may be available in a nightly release! fixed-in-net9.0-nightly This may be available in a nightly release!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants