Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

[Aapt2] _CreateAapt2VersionCache slow due to wildcard usage #2121

Closed
jonathanpeppers opened this issue Aug 30, 2018 · 1 comment
Closed

[Aapt2] _CreateAapt2VersionCache slow due to wildcard usage #2121

jonathanpeppers opened this issue Aug 30, 2018 · 1 comment
Assignees
@dellis1972 @jonathanpeppers
Labels
Area: App+Library Build Issues when building Library projects or Application projects. Area: Performance Issues with performance. bug Component does not function as intended.

Comments

@jonathanpeppers
Copy link
Member

Steps to Reproduce

  1. I built the tests/Xamarin.Forms-Performance-Integration app
  2. Modify a XAML file, build the app again
  3. _CreateAapt2VersionCache took 181ms, but the values were cached!

I think the problem is due to wildcard usage:

<ItemGroup>
	<_CompiledFlataArchive Include="$(IntermediateOutputPath)\**\*.flata"/>
	<_CompiledFlataStamp Include="$(IntermediateOutputPath)\**\compiled.stamp"/>
</ItemGroup>

Maybe these could be done without using **? we have a lot of files in here.

Expected Behavior

_CreateAapt2VersionCache should take near 1ms when things are cached.

Actual Behavior

_CreateAapt2VersionCache is taking ~181ms even when cached.

Version Information

Was using master as of: 0a46788

Log File

msbuild.zip
@jonathanpeppers jonathanpeppers added bug Component does not function as intended. Area: App+Library Build Issues when building Library projects or Application projects. Area: Performance Issues with performance. labels Aug 30, 2018
@dellis1972
Copy link
Contributor

dellis1972 commented Aug 30, 2018 •
edited
Loading

We should only run the entire target if Condition="'$(_Aapt2Version)' != '@(_Aapt2VersionCache)'" .
But we might need conditions on the ItemGroup entires too, since I believe even if a target is skipped the ItemGroups and Properties are evaluated.

We need the wildcards since we will need to recurse into subfolders like the lp ones.. That said, we might be able to optimise by looking at specific sub dirs. So we don't end up going through things like the android directory, or the linksrc ones

$(IntermediateOutputPath)\lp\**\*.flata
$(IntermediateOutputPath)\*.flata

dellis1972 added a commit to dellis1972/xamarin-android that referenced this issue Aug 31, 2018
@dellis1972
[Xamarin.Android.Build.Tasks] _CreateAapt2VersionCache slow due to …
2211528 
…wildcard usage

Fixes dotnet#2121

There were a number of performace issues with `_CreateAapt2VersionCache`.
First the wildcards were processing the ENTIRE `$(IntermediateOutputPath)`!
What they should have been doing was targeting specific
directories. I.e the root of `$(IntermediateOutputPath)` and
the directories under `$(IntermediateOutputPath)\lp`.

The target did not have a Condition to stop it running if
the versions matched, so that has been added.

However, even if a target is NOT run. MSbuild will still evaluate
the `PropertyGroups` and `ItemGroups` within the targets. So we
need to add `Conditon` on the `_CompiledFlataArchive` and
`_CompiledFlataStamp` items as well.

With these in place the time when this target is skipped is down
to 1ms.
@dellis1972 dellis1972 mentioned this issue Aug 31, 2018
Merged
dellis1972 added a commit to dellis1972/xamarin-android that referenced this issue Sep 4, 2018
@dellis1972
[Xamarin.Android.Build.Tasks] _CreateAapt2VersionCache slow due to …
215f26c 
…wildcard usage

Fixes dotnet#2121

There were a number of performace issues with `_CreateAapt2VersionCache`.
First the wildcards were processing the ENTIRE `$(IntermediateOutputPath)`!
What they should have been doing was targeting specific
directories. I.e the root of `$(IntermediateOutputPath)` and
the directories under `$(IntermediateOutputPath)\lp`.

The target did not have a Condition to stop it running if
the versions matched, so that has been added.

However, even if a target is NOT run. MSbuild will still evaluate
the `PropertyGroups` and `ItemGroups` within the targets. So we
need to add `Conditon` on the `_CompiledFlataArchive` and
`_CompiledFlataStamp` items as well.

With these in place the time when this target is skipped is down
to 1ms.
dellis1972 added a commit to dellis1972/xamarin-android that referenced this issue Sep 5, 2018
@dellis1972
[Xamarin.Android.Build.Tasks] _CreateAapt2VersionCache slow due to …
04be9da 
…wildcard usage

Fixes dotnet#2121

There were a number of performace issues with `_CreateAapt2VersionCache`.
First the wildcards were processing the ENTIRE `$(IntermediateOutputPath)`!
What they should have been doing was targeting specific
directories. I.e the root of `$(IntermediateOutputPath)` and
the directories under `$(IntermediateOutputPath)\lp`.

The target did not have a Condition to stop it running if
the versions matched, so that has been added.

However, even if a target is NOT run. MSbuild will still evaluate
the `PropertyGroups` and `ItemGroups` within the targets. So we
need to add `Conditon` on the `_CompiledFlataArchive` and
`_CompiledFlataStamp` items as well.

With these in place the time when this target is skipped is down
to 1ms.
jonpryor pushed a commit that referenced this issue Sep 5, 2018
@dellis1972 @jonpryor
[Xamarin.Android.Build.Tasks] _CreateAapt2VersionCache slow due to …
35ad986 
…wildcard usage (#2124)

Fixes: #2121

There were a number of performance issues with the
`_CreateAapt2VersionCache` target.

Firstly the wildcards were processing the *entire*
`$(IntermediateOutputPath)` tree!  What they should have been doing
was targeting specific directories, e.g. the root of
`$(IntermediateOutputPath)` and the directories under
`$(IntermediateOutputPath)\lp`.

Secondly the target did not have a `Condition` to stop it from
running if the versions matched, so that has been added.

Thirdly, even if a target is NOT run, MSBuild will still evaluate the
`<PropertyGroup/>`s and `<ItemGroup/>`s within the Target, so we need
to add a `Conditon` on the `@(_CompiledFlataArchive)` and
`@(_CompiledFlataStamp)` items as well to prevent evaluation and
corresponding directory traversal.

With these in place the time when this target is reduced to 1ms.
@ghost ghost locked as resolved and limited conversation to collaborators Jun 7, 2022
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Assignees

@dellis1972 dellis1972

@jonathanpeppers jonathanpeppers

Labels
Area: App+Library Build Issues when building Library projects or Application projects. Area: Performance Issues with performance. bug Component does not function as intended.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@dellis1972 @jonathanpeppers