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] simplify inputs to _CompileToDalvik #2131

Merged
merged 1 commit into from
Sep 3, 2018

Conversation

jonathanpeppers
Copy link
Member

@jonathanpeppers jonathanpeppers commented Aug 31, 2018

Recently I've been discovering more and more places where usage of
**\*.* can hurt the performance of our build.

In this case, the _FindCompiledJavaFiles target:

<Target Name="_FindCompiledJavaFiles" DependsOnTargets="_CompileJava">
    <CreateItem Include="$(IntermediateOutputPath)android\bin\classes\\**\*.class">
        <Output TaskParameter="Include" ItemName="_CompiledJavaFiles" />
    </CreateItem>
</Target>

This target is running even in builds with no changes, so we are
always recursing directories and finding *.class files.

But since we are using a classes.zip file now, we don't need to
recurse and find the *.class files at all anymore!

I was able to remove this target completely, and change
$(_CompileToDalvikInputs) to use
$(IntermediateOutputPath)android\bin\classes.zip. Less inputs also
help build times, because MSBuild won't have to evaluate timestamps on
all the *.class files. It can just look at the single classes.zip
file.

To see the difference, a build with no changes was taking:

46 ms  _FindCompiledJavaFiles                     1 calls
15 ms  _CompileToDalvikWithDx                     1 calls

After these changes:

4 ms  _CompileToDalvikWithDx                     1 calls

_FindCompiledJavaFiles is gone completely.

I also made sure to include _CompileJava in
$(_CompileToDalvikDependsOnTargets) in place of
_FindCompiledJavaFiles.

@jonathanpeppers
Copy link
Member Author

jonathanpeppers commented Aug 31, 2018

Build logs: FindCompiledJavaFiles.zip

Look at the updated ones below

@jonathanpeppers
Copy link
Member Author

Oops this is important: DependsOnTargets="_CompileJava"

Fixing it...

Recently I've been discovering more and more places where usage of
`**\*.*` can hurt the performance of our build.

In this case, the `_FindCompiledJavaFiles` target:

    <Target Name="_FindCompiledJavaFiles" DependsOnTargets="_CompileJava">
        <CreateItem Include="$(IntermediateOutputPath)android\bin\classes\\**\*.class">
            <Output TaskParameter="Include" ItemName="_CompiledJavaFiles" />
        </CreateItem>
    </Target>

This target is running even in builds with no changes, so we are
*always* recursing directories and finding `*.class` files.

But since we are using a `classes.zip` file now, we don't need to
recurse and find the `*.class` files at all anymore!

I was able to remove this target completely, and change
`$(_CompileToDalvikInputs)` to use
`$(IntermediateOutputPath)android\bin\classes.zip`. Less inputs also
help build times, because MSBuild won't have to evaluate timestamps on
all the `*.class` files. It can just look at the single `classes.zip`
file.

To see the difference, a build with no changes was taking:

    46 ms  _FindCompiledJavaFiles                     1 calls
    15 ms  _CompileToDalvikWithDx                     1 calls

After these changes:

    4 ms  _CompileToDalvikWithDx                     1 calls

`_FindCompiledJavaFiles` is gone completely.

I also made sure to include `_CompileJava` in
`$(_CompileToDalvikDependsOnTargets)` in place of
`_FindCompiledJavaFiles`.
@jonathanpeppers
Copy link
Member Author

After fixing the missing dependency for _CompileJava, the improvements here aren't as good as I initially saw. This likely helps about 50ms for all builds (even with no changes).

Updated logs here: FindCompiledJavaFiles.zip

@dellis1972 dellis1972 merged commit 2598be5 into dotnet:master Sep 3, 2018
jonpryor pushed a commit that referenced this pull request Sep 5, 2018
)

Recently I've been discovering more and more places where usage of
`**\*.*` can hurt the performance of our build.

In this case, the `_FindCompiledJavaFiles` target:

    <Target Name="_FindCompiledJavaFiles" DependsOnTargets="_CompileJava">
        <CreateItem Include="$(IntermediateOutputPath)android\bin\classes\\**\*.class">
            <Output TaskParameter="Include" ItemName="_CompiledJavaFiles" />
        </CreateItem>
    </Target>

This target is running even in builds with no changes, so we are
*always* recursing directories and finding `*.class` files.

But since we are using a `classes.zip` file now, we don't need to
recurse and find the `*.class` files at all anymore!

I was able to remove this target completely, and change
`$(_CompileToDalvikInputs)` to use
`$(IntermediateOutputPath)android\bin\classes.zip`. Less inputs also
help build times, because MSBuild won't have to evaluate timestamps on
all the `*.class` files. It can just look at the single `classes.zip`
file.

To see the difference, a build with no changes was taking:

    46 ms  _FindCompiledJavaFiles                     1 calls
    15 ms  _CompileToDalvikWithDx                     1 calls

After these changes:

    4 ms  _CompileToDalvikWithDx                     1 calls

`_FindCompiledJavaFiles` is gone completely.

I also made sure to include `_CompileJava` in
`$(_CompileToDalvikDependsOnTargets)` in place of
`_FindCompiledJavaFiles`.
@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