Skip to content

[perf] guard the _AdditionalJavaStubDirectory glob in _FindJavaStubFiles - #12287

Open
jonathanpeppers wants to merge 1 commit into
mainfrom
jonathanpeppers-build-perf-findjavastubfiles
Open

[perf] guard the _AdditionalJavaStubDirectory glob in _FindJavaStubFiles#12287
jonathanpeppers wants to merge 1 commit into
mainfrom
jonathanpeppers-build-perf-findjavastubfiles

Conversation

@jonathanpeppers

@jonathanpeppers jonathanpeppers commented Jul 31, 2026

Copy link
Copy Markdown
Member

Fixes a per-build cost in _FindJavaStubFiles that fires on every build, including a fully no-op incremental build.

Root cause

MSBuild %() batching over an empty item list still evaluates the Include once, with an empty %(Identity). So this line:

<_JavaStubFiles Include="%(_AdditionalJavaStubDirectory.Identity)**\*.java" />

degrades to Include="**\*.java" — relative to the project directory — whenever @(_AdditionalJavaStubDirectory) is empty. That is the default case, since $(_AndroidTypeMapImplementation) defaults to llvm-ir.

Consequences on a dotnet new maui -sc app (Debug, CoreCLR, arm64-v8a):

Tree walked by the stray glob 4,537 files / 1,824 directories (bin, obj, Platforms, ...)
@(_JavaStubFiles) items 866 instead of 433 — every android/src stub added twice
@(FileWrites) same duplication
Extra junk .java from other intermediate output paths (e.g. a stale non-RID obj/.../android/src) leaking into _CompileJava Inputs

Verified directly: with _AdditionalJavaStubDirectory empty, a Bogus.java dropped anywhere under the project directory shows up in @(_JavaStubFiles).

Fix

Guard the batched ItemGroup on '@(_AdditionalJavaStubDirectory->Count())' != '0'. No Inputs/Outputs added — this stays an item-population target, as _CompileJava requires.

Numbers

Interleaved A/B, 6 pairs, -nodeReuse:false, -clp:PerformanceSummary, _FindJavaStubFiles target time. Every run was asserted to be a true no-op (CoreCompile and _CompileJava both logged as skipped); total build ~4.6–5.0 s.

raw (ms)
before 98, 200, 218, 97, 96, 95
after 5, 11, 4, 4, 4, 4
paired delta 93, 189, 214, 93, 92, 91

Ranges are disjoint. The before value is genuinely variable run-to-run because it is dominated by a filesystem walk of the whole project directory — it scales with how much is sitting under bin//obj/, not with the number of Java stubs. Separate verified-no-op runs on the same machine landed at 83/117/84 ms before vs 5/4/4 after. Treat the win as "tens to a couple hundred ms, proportional to project tree size", not a single fixed number.

That scaling is the part worth caring about: the cost grows with accumulated build output, so it gets worse the longer a developer works without cleaning — the opposite of what an inner-loop target should do. The after side is the stable one: 4–11 ms regardless of tree size.

Correctness

@(_JavaStubFiles) dumped from a real build on a clean app tree, sorted, diffed:

typemap before after set diff
llvm-ir (empty _AdditionalJavaStubDirectory) 866 items / 433 distinct 433 / 433 0
trimmable (populated) 753 / 753 753 / 753 0

Incrementality, checked with Skipping target "_CompileJava":

  • no-op rebuild → _CompileJava skipped
  • touch a stub in android/src → runs
  • add a .java in an _AdditionalJavaStubDirectory → runs
  • touch that .java → runs
  • remove it → not detected — identical before and after; inherent to MSBuild Inputs/Outputs, not a regression

Tests

  • Microsoft.Android.Build.BaseTasks-Tests: 129 passed
  • Targeted Xamarin.Android.Build.Tests (JavacTaskDoesNotRunOnSecondBuild, GenerateJavaStubsAndAssembly, TrimmableTypeMapBuildTests Java cases): 6 passed / 2 skipped / 2 failed — the same two (Build_WithTrimmableTypeMap_DeletesStaleGeneratedJavaSources, Build_WithTrimmableTypeMap_RecompilesUpdatedGeneratedJavaSources) fail identically without this change, verified by swapping the target back and re-running.

Independent fix, targets main.

Copilot AI review requested due to automatic review settings July 31, 2026 19:51

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves MSBuild incremental-build performance by preventing _FindJavaStubFiles from accidentally globbing the entire project directory when @(_AdditionalJavaStubDirectory) is empty (the common case for llvm-ir typemap).

Changes:

  • Splits _FindJavaStubFiles item population so the _AdditionalJavaStubDirectory glob is only evaluated when that item list is non-empty.
  • Adds an explanatory comment documenting why the guard is necessary (empty-item batching still evaluates Include once).

Comment thread src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets Outdated
`_FindJavaStubFiles` cost 137-171 ms on a *no-op* incremental build of a
`dotnet new maui -sc` app (Debug, CoreCLR, arm64-v8a).

The cause is MSBuild `%()` batching semantics: batching over an *empty*
item list still evaluates the `Include` once with an empty `%(Identity)`,
so

    <_JavaStubFiles Include="%(_AdditionalJavaStubDirectory.Identity)**\*.java" />

degrades to `Include="**\*.java"` relative to the project directory whenever
`@(_AdditionalJavaStubDirectory)` is empty -- which is the default, since
`$(_AndroidTypeMapImplementation)` defaults to `llvm-ir`.

That stray glob recursively walks the *entire* app tree (`bin`, `obj`,
`Platforms`, ...; 4,537 files / 1,824 directories in the benchmark app) on
every build, and re-adds every `android/src` stub a second time -- 866
items instead of 433, duplicated into `@(FileWrites)` as well. It also
picks up unrelated `.java` under other intermediate output paths (e.g. a
stale non-RID `obj/.../android/src`), which do not belong in `_CompileJava`
`Inputs`.

Guarding the batched `ItemGroup` on `'@(_AdditionalJavaStubDirectory->Count())' != '0'`
skips it entirely when there is nothing to glob.

Interleaved A/B, 7 pairs, no-op incremental app build, `-nodeReuse:false`,
`-clp:PerformanceSummary`, `_FindJavaStubFiles` target time:

    before: 143, 137, 138, 155, 171, 162, 141 ms
    after:    5,   4,   5,   5,   5,   6,   5 ms

Correctness: `@(_JavaStubFiles)` dumped from a real build on a clean tree.

    llvm-ir   before 866 items / 433 distinct, after 433 / 433, set diff 0
    trimmable before 753 items / 753 distinct, after 753 / 753, set diff 0

`_CompileJava` still skips on a no-op rebuild, still runs after touching a
stub in `android/src` and after adding or touching a `.java` in an
`_AdditionalJavaStubDirectory`. (File *removal* is not detected either
before or after -- an inherent limit of MSBuild `Inputs`/`Outputs`.)

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 4ac21e04-ebb5-47cb-9425-33211d2649f4
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@jonathanpeppers
jonathanpeppers force-pushed the jonathanpeppers-build-perf-findjavastubfiles branch from 323319f to 8dc218d Compare July 31, 2026 19:57
@jonathanpeppers
jonathanpeppers changed the base branch from jonathanpeppers-build-perf-coreclr-round2 to main July 31, 2026 19:57
@jonathanpeppers jonathanpeppers added the ready-to-review This PR is ready to review/merge, I think any CI failures are just flaky (ignorable). label Jul 31, 2026
@jonathanpeppers
jonathanpeppers enabled auto-merge (squash) July 31, 2026 22:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-review This PR is ready to review/merge, I think any CI failures are just flaky (ignorable).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants