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] remove unused Inputs/Outputs #2309

Merged
merged 1 commit into from
Oct 17, 2018

Conversation

jonathanpeppers
Copy link
Member

While reviewing our MSBuild targets, I noticed three targets that
declared Inputs and Outputs, but did not actually update any
files:

  • _CollectConfigFiles
  • _CollectMdbFiles
  • _CollectPdbFiles

This means that MSBuild was unnecessarily looking at files to see if
they exist, and might be comparing timestamps as well. The targets
would also always run, so it doesn't matter to have Inputs and
Outputs in these targets at all!

I basically removed these Inputs and Outputs, and saw a
performance gain with just doing that. I also saw performance gains to
targets that depend on these three targets. Since Inputs and
Outputs timing is internal to MSBuild, I am not sure where the
actual time is being tallied here.

Before:
    1 ms  _CopyConfigFiles                           1 calls
    3 ms  _CopyMdbFiles                              2 calls
    3 ms  _CollectMdbFiles                           1 calls
    4 ms  _CopyPdbFiles                              1 calls
    5 ms  _CollectPdbFiles                           1 calls
    5 ms  _CollectConfigFiles                        1 calls
   20 ms  _ConvertPdbFiles                           1 calls

A total of ~46ms

After:
    0 ms  _ConvertPdbFiles                           1 calls
    0 ms  _CopyPdbFiles                              1 calls
    1 ms  _CollectMdbFiles                           1 calls
    1 ms  _CopyMdbFiles                              2 calls
    1 ms  _CopyConfigFiles                           1 calls
    2 ms  _CollectConfigFiles                        1 calls
    5 ms  _CollectPdbFiles                           1 calls

A total of ~10ms

An overall improvement of ~36ms.

This was the samples/HelloWorld project in this repo, a build with
no changes. I suspect the improvement would be even better for larger
projects.

While reviewing our MSBuild targets, I noticed three targets that
declared `Inputs` and `Outputs`, but did not actually update any
files:

- `_CollectConfigFiles`
- `_CollectMdbFiles`
- `_CollectPdbFiles`

This means that MSBuild was unnecessarily looking at files to see if
they exist, and might be comparing timestamps as well. The targets
would also _always_ run, so it doesn't matter to have `Inputs` and
`Outputs` in these targets at all!

I basically removed these `Inputs` and `Outputs`, and saw a
performance gain with just doing that. I also saw performance gains to
targets that depend on these three targets. Since `Inputs` and
`Outputs` timing is internal to MSBuild, I am not sure where the
actual time is being tallied here.

    Before:
        1 ms  _CopyConfigFiles                           1 calls
        3 ms  _CopyMdbFiles                              2 calls
        3 ms  _CollectMdbFiles                           1 calls
        4 ms  _CopyPdbFiles                              1 calls
        5 ms  _CollectPdbFiles                           1 calls
        5 ms  _CollectConfigFiles                        1 calls
       20 ms  _ConvertPdbFiles                           1 calls

A total of ~46ms

    After:
        0 ms  _ConvertPdbFiles                           1 calls
        0 ms  _CopyPdbFiles                              1 calls
        1 ms  _CollectMdbFiles                           1 calls
        1 ms  _CopyMdbFiles                              2 calls
        1 ms  _CopyConfigFiles                           1 calls
        2 ms  _CollectConfigFiles                        1 calls
        5 ms  _CollectPdbFiles                           1 calls

A total of ~10ms

An overall improvement of ~36ms.

This was the `samples/HelloWorld` project in this repo, a build with
no changes. I suspect the improvement would be even better for larger
projects.
@jonathanpeppers
Copy link
Member Author

Build logs for this change: logs.zip

@dellis1972
Copy link
Contributor

@jonathanpeppers looks like I was a bit overzealous with the Inputs and Outputs then :) This looks good.

@dellis1972 dellis1972 merged commit ca42832 into dotnet:master Oct 17, 2018
@github-actions github-actions bot locked and limited conversation to collaborators Feb 2, 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.

2 participants