[TrimmableTypeMap] Improve trimmable typemap build incrementality#11764
[TrimmableTypeMap] Improve trimmable typemap build incrementality#11764simonrozsival wants to merge 6 commits into
Conversation
The trimmable typemap generator (`_GenerateTrimmableTypeMap`) and the JCW copy (`_GenerateJavaStubs`) had three incrementality gaps: * The generated TypeMap DLLs are written with `CopyIfStreamChanged`, so an unchanged assembly keeps its old timestamp. Because those DLLs were the only `Outputs`, the target's inputs were always newer than its (untouched) outputs and it re-ran on every build. Add a dedicated `_GenerateTrimmableTypeMap.stamp` that is always touched, so the target is correctly skipped on no-op builds, and use it as the `_GenerateJavaStubs` sentinel. * `@(PrivateSdkAssemblies)` and `@(FrameworkAssemblies)` were not declared as inputs even though they can contribute managed↔Java mappings, so changes in them did not trigger regeneration. * Removing a managed type left its stale JCW `.java` (and compiled `.class`) behind. `GenerateTrimmableTypeMap` now reports `DeletedJavaFiles`; the target mirrors the deletion into `android/src` and busts the Java compile stamp so `_CompileJava` drops the stale class. Also skip `_GenerateTrimmableTypeMap` in design-time builds (project references may resolve to paths not produced when `SkipCompilerExecution=true`, and the output is not needed for IDE information); combined with the existing inner per-RID guard the generator runs exactly once per build. JCWs are now copied with `SkipUnchangedFiles="true"`. Add a `README.md` documenting the trimmable typemap build pipeline and its incrementality design, and build tests covering stale-JCW pruning and copying updated JCWs when the typemap assemblies are unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR improves MSBuild incrementality for the trimmable typemap pipeline (_AndroidTypeMapImplementation=trimmable) by introducing a stable stamp-based incremental sentinel, expanding the generator’s declared inputs, and ensuring stale generated Java artifacts are removed so downstream Java compilation stays correct.
Changes:
- Add new build tests covering stale-Java deletion and updated-Java copying behavior for the trimmable typemap pipeline.
- Extend
GenerateTrimmableTypeMapto report deleted stale generated*.javasources (DeletedJavaFiles) and delete them from the generator output directory. - Update
Microsoft.Android.Sdk.TypeMap.Trimmable.targetsto:- Use a dedicated generator stamp as an incremental sentinel,
- Include additional assembly inputs (
@(PrivateSdkAssemblies),@(FrameworkAssemblies)), - Mirror deleted Java sources into
android/srcand force Java recompilation when needed, - Copy JCWs with
SkipUnchangedFiles="true".
- Add a README describing the pipeline and incrementality design.
Show a summary per file
| File | Description |
|---|---|
| src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/TrimmableTypeMapBuildTests.cs | Adds tests validating incremental behavior, Java source copying, and stale Java/class cleanup. |
| src/Xamarin.Android.Build.Tasks/Tasks/GenerateTrimmableTypeMap.cs | Adds stale generated Java cleanup and exposes deletions via a new MSBuild output item group. |
| src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.TypeMap.Trimmable.targets | Implements stamp-based incrementality, expands generator inputs, mirrors deletions into android/src, and improves copy incrementality. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/README.md | Documents the trimmable typemap pipeline and its incrementality approach. |
Copilot's findings
- Files reviewed: 4/4 changed files
- Comments generated: 2
| <!-- Touch the generated assemblies, the list file, and the stamp so the target's | ||
| Outputs are always newer than its Inputs even when CopyIfStreamChanged left an | ||
| unchanged DLL untouched. This is what makes the target skip on no-op builds. --> | ||
| <Touch Files="@(_GeneratedTypeMapAssemblies);$(_TypeMapAssembliesListFile);$(_TrimmableTypeMapOutputStamp)" AlwaysCreate="true" /> |
There was a problem hiding this comment.
Is it possible to only touch the $(_TrimmableTypeMapOutputStamp) file and make it the only Output?
Or are @(_GeneratedTypeMapAssemblies);$(_TypeMapAssembliesListFile) Inputs to another target? -- that would be the only reason to touch so many files.
There was a problem hiding this comment.
Good call — done in 8a70372. _GenerateTrimmableTypeMap now declares only $(_TrimmableTypeMapOutputStamp) as its Outputs and touches only that stamp. The generated TypeMap DLLs and typemap-assemblies.txt are written via CopyIfStreamChanged (stable timestamps when unchanged) and are not consumed as timestamp Inputs by any other target, so they no longer need to be touched.
Add two build tests for the CoreCLR + PublishTrimmed trimmable typemap path, where the JCWs that get compiled and packaged come from the post-trim `typemap/linked-java` directory: * `..._KeepsAndroidSrcConsistentWithLinkedJava` asserts the correctness invariant that every linked-java JCW has an identical copy under `android/src`, after both the initial build and a no-op rebuild. * `..._PostTrimJavaGenerationIsIncremental` asserts that a no-op rebuild skips `_GeneratePostTrimTrimmableTypeMapJavaSources` and `_GenerateJavaStubs`. It is currently `[Ignore]`d because post-trim Java generation runs on every build today; it documents the incrementality gap to fix (make post-trim incremental and key `_GenerateJavaStubs` off `_TrimmableJavaSourceStamp`). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Addresses review feedback on the _GenerateJavaStubs incremental sentinel and fixes the underlying reason it was hard to get right: post-trim Java generation ran on every Release CoreCLR build. Root cause: _GeneratePostTrimTrimmableTypeMapJavaSources declared `@(ResolvedFileToPublish)` as its Inputs, which also contains non-assembly publish outputs (e.g. UnnamedProject.runtimeconfig.json) whose paths do not exist when the target runs. MSBuild treats a non-existent Input as out-of-date, so the target ran every build, wiped and regenerated `typemap/linked-java`, and forced downstream copying. Changes: * Compute the post-trim input assemblies in a new `_ComputePostTrimTrimmableTypeMapInputs` target, filtered to `.dll` files that exist on disk, and use that item as the target's Inputs. The target now skips on a no-op rebuild. * `_GenerateJavaStubs` keys its incrementality off `$(_TrimmableJavaSourceStamp)` (the post-trim stamp for CoreCLR + PublishTrimmed, the generator stamp otherwise) instead of the generator stamp alone, so it reacts to post-trim JCW regeneration while staying incremental (review feedback from @Copilot). The non-trim default of `_TrimmableJavaSourceStamp` is redefined from the TypeMap DLL to the generator stamp. * Re-emit the post-trim `linked-java` outputs (and their android/src copies and stamp) from `_RecordTrimmableTypeMapFileWrites` so MSBuild's IncrementalClean does not delete them on builds where post-trim is skipped. * `_GenerateTrimmableTypeMap` now declares only the stamp as its `Outputs` and touches only the stamp; the generated DLLs/list are written via CopyIfStreamChanged and are not consumed as timestamp Inputs elsewhere (review feedback from @jonathanpeppers). * Re-enable Build_WithTrimmableTypeMap_PublishTrimmed_PostTrimJavaGenerationIsIncremental (previously [Ignore]d); it now passes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Addresses code-review feedback that the PublishTrimmed path could leave stale Java sources in android/src. On CoreCLR + PublishTrimmed, the JCWs that get compiled and packaged are copied into android/src from the post-trim `typemap/linked-java` directory. When a type is trimmed away between builds, _GeneratePostTrimTrimmableTypeMapJavaSources regenerates linked-java without that type's JCW, but `_GenerateJavaStubs` only copies (with SkipUnchangedFiles) and never removed the now-orphaned android/src copy — so a stale .java (and its .class) could be compiled into the app. The existing stale-pruning only covered the non-trim `typemap/java` pass. Fix, symmetric with the non-trim pass: * GenerateTrimmableTypeMap: when CleanJavaSourceOutputDirectory is set (the post-trim pass wipes its output dir before regenerating), snapshot the previous *.java set before the wipe and report DeletedJavaFiles as (previous - regenerated). This keeps the deletion precise — only files the generator itself previously produced are reported, never unrelated android/src sources like ApplicationRegistration.java. Replaces the post-write rescan in the clean case (it could never find anything in a freshly-wiped dir). * _GeneratePostTrimTrimmableTypeMapJavaSources: consume DeletedJavaFiles, mirror the deletions into the android/src copies, and bust the Java compile stamp so _CompileJava drops the stale class — reusing the block from _GenerateTrimmableTypeMap. Also from review: * Add Build_WithTrimmableTypeMap_PublishTrimmed_DeletesStaleAndroidSrcWhenLinkedJavaShrinks (fails before this fix, passes after). * Make AssertAndroidSrcMatchesLinkedJava derive both directories from GetIntermediaryPath instead of walking parents of a recursively-found dir, and drop the post-Assert.IsNotNull nullable reliance. * Remove the redundant BeforeTargets on _ComputePostTrimTrimmableTypeMapInputs (ordering is already guaranteed by the consumer's DependsOnTargets). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Update the incrementality README to describe stale-JCW pruning on both generator passes (pre-trim scan vs. post-trim snapshot diff) and the invariant that android/src contains exactly the JCWs the active pass produces — no missing and no stale files. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Follow-up polish from self-review of the post-trim android/src pruning: * Route clean mode (the post-trim pass) through the snapshot-diff path consistently, including the first run where there is nothing to wipe (empty snapshot), instead of falling back to the directory-scan branch. * Skip the snapshot diff when generation has logged errors. If CopyJavaSourcesFromInputDirectory dropped a source (XA4255), GeneratedJavaFiles is incomplete and the prior-minus-current diff could otherwise flag a still-valid JCW for deletion. The build already fails on logged errors, so no pruning should happen. * Extend Build_WithTrimmableTypeMap_PublishTrimmed_DeletesStaleAndroidSrcWhenLinkedJavaShrinks to also plant a stale android/bin/classes .class and assert it is dropped — covering the compile-stamp bust that the fix relies on. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
/review |
Summary
Makes the trimmable typemap build pipeline (
_AndroidTypeMapImplementation=trimmable, used by CoreCLR and NativeAOT) fully incremental, skipped in design‑time builds, and self‑consistent on every rebuild. Extracted as a focused, well‑tested slice from the larger #11617, and implemented on top ofmain's current generator/task API rather than the runtime rewrite in that PR.The bulk of this description is a deep dive into how incrementality is achieved, because the pipeline has several non‑obvious moving parts (two generator passes, content‑based writes, dynamic outputs vs.
IncrementalClean, and a post‑link Java regeneration step).Background: what the trimmable typemap generator produces
The legacy typemap implementations (
llvm-ir,managed) embed the managed ↔ Java type mapping into native binaries. The trimmable implementation instead generates, from a scan of the app's assemblies:_<Assembly>.TypeMap.dllper input assembly plus a_Microsoft.Android.TypeMaps.dllroot — that are trimmer‑friendly (unused entries are removed by the IL linker), and*.java, "JCW") sources that Java needs to call back into managed code, plusacw-map.txtand a mergedAndroidManifest.xml.This is driven by the
GenerateTrimmableTypeMapMSBuild task. The active runtime is selected by$(_AndroidRuntime)(CoreCLRorNativeAOT); runtime‑specific targets extend the shared pipeline (…TypeMap.Trimmable.CoreCLR.targets,…TypeMap.Trimmable.NativeAOT.targets).The pipeline (target graph)
Shared / Debug CoreCLR & NativeAOT (no trimming)
_GenerateJavaStubsoverrides the legacy target of the same name fromBuildOrder.targets; on the trimmable path the JCWs already exist, so it only copies them into$(IntermediateOutputPath)android/srcand wires up the manifest/acw-map.txt/native config.Release CoreCLR (
PublishTrimmed=true) — two generator passesThe post‑trim pass exists because, after trimming, the set of types that still need JCWs is a subset of the pre‑trim set.
_GenerateJavaStubssources its JCWs from$(_TypeMapJavaStubsSourceDirectory), which istypemap/linked-javafor CoreCLR +PublishTrimmedandtypemap/javaotherwise.NativeAOT (
PublishTrimmed=trueas well) does not use the post‑trim Java pass — it feeds the pre‑trim JCWs to ILC and its own native steps — so for NativeAOT the sources staytypemap/java.How incrementality is achieved (deep dive)
The pipeline follows the repo's MSBuild best practices: every expensive target declares
Inputs/Outputs, re‑emits its dynamicFileWrites, and uses stamp files where a real output can't serve as a reliable timestamp sentinel. There are six principles at work.1. Stamp files are the incremental sentinels — because the real outputs are content‑addressed
GenerateTrimmableTypeMapwrites every output withFiles.CopyIfStreamChanged(and the per‑assemblyWriteAssembliesToDiskadditionally compares timestamps): an output whose content did not change keeps its old timestamp. That's exactly what you want to avoid churning downstream Java/native compilation — but it makes those files unusable as MSBuildOutputs, because their timestamps can be older than theInputseven on a successful run, which makes MSBuild consider the target perpetually out‑of‑date and re‑run it every build.So
_GenerateTrimmableTypeMapdeclares a dedicated stamp as its sole output and touches only that stamp:The stamp is always touched on a run and never touched on a skip, so the target is correctly skipped iff none of its inputs changed. The generated DLLs and
typemap-assemblies.txtare not touched and are not declared as outputs — they are content‑addressed, and no other target consumes them as timestampInputs(verified), so touching them would be pure churn. (This is the@jonathanpeppersreview point.)_GenerateJavaStubsuses the analogous idea, but its sentinel must reflect whichever pass produced the JCWs it copies:where
_TrimmableJavaSourceStampresolves to the post‑trim stamp for CoreCLR +PublishTrimmed, and the pre‑trim generator stamp otherwise. This is what lets_GenerateJavaStubsreact to post‑trim JCW regeneration while staying incremental on no‑op builds. (This is the@Copilotreview point; the non‑trim default was previously the TypeMap DLL, whose timestamp is unreliable for the reason above.)2.
Inputsmust reference files that actually existMSBuild treats a non‑existent
Inputsfile as "out of date" and runs the target. The post‑trim pass originally declaredInputs="@(ResolvedFileToPublish)", which also lists non‑assembly publish outputs — e.g.…/bin/Release/<rid>/UnnamedProject.runtimeconfig.json— whose paths do not exist when the target runs. That single phantom input made_GeneratePostTrimTrimmableTypeMapJavaSourcesrun on every build, wiping and regeneratinglinked-javaeach time.The fix factors input collection into
_ComputePostTrimTrimmableTypeMapInputs, which keeps only.dllitems that exist on disk:so the post‑trim pass is now genuinely incremental and its stamp is a trustworthy sentinel for
_GenerateJavaStubs.3. Dynamic outputs must be re‑published to
@(FileWrites)on skipped buildsThe set of generated assemblies and JCWs is data‑dependent (it comes from scanning), so the
ItemGroups that register those files into@(FileWrites)live inside the generating targets — and therefore don't execute on a no‑op build where those targets are skipped. If nothing re‑declares them, MSBuild'sIncrementalCleansees the previously‑tracked files as orphaned and deletes them before packaging reads its inputs._RecordTrimmableTypeMapFileWritesruns unconditionally (gated only on the assemblies‑list file existing) and re‑emits the previous dynamic outputs back into@(FileWrites)beforeIncrementalClean:typemap-assemblies.txt),typemap/java) and theirandroid/srccopies,typemap/linked-java) and theirandroid/srccopies, plus the post‑trim stamp, andacw-map.txt,ApplicationRegistration.java, and the generator stamp.The post‑trim entries are new in this PR: once
_GeneratePostTrimTrimmableTypeMapJavaSourcesbecame skippable (principle 2), itslinked-javaoutputs would otherwise be deleted byIncrementalCleanon the builds where it's skipped. The globs are empty for configurations that have no post‑trim pass, so this is a no‑op there.4. Content‑based writes + explicit stale pruning keep
android/srcexactBecause generation is content‑addressed (
CopyIfStreamChanged) and theandroid/srccopy usesSkipUnchangedFiles="true", an unchanged JCW never updates a timestamp and never triggers Java recompilation.The flip side is removal: when a JCW disappears between builds — a type deleted from source, or trimmed away on the
PublishTrimmedpath — itsandroid/srccopy must not linger, or it would be compiled and packaged. Both generator passes report the JCWs they no longer produce asDeletedJavaFiles; the owning target mirrors each deletion into theandroid/srccopy and, if anything was deleted, deletes$(_AndroidCompileJavaStampFile)so_CompileJavare‑runs and drops the stale.class:The two passes compute the deleted set differently because of how each manages its output directory:
_GenerateTrimmableTypeMap, writingtypemap/java): the task scans the output dir and deletes any*.javathe current pass didn't produce._GeneratePostTrimTrimmableTypeMapJavaSources, writingtypemap/linked-javawithCleanJavaSourceOutputDirectory=true): the dir is wiped before regeneration, so the task snapshots the previous*.javaset before the wipe and reportsprevious − regenerated. This keeps the deletion precise — only files the generator itself previously produced are ever removed fromandroid/src, never unrelated sources likeApplicationRegistration.java.The net invariant is two‑directional:
android/srccontains exactly the JCWs the active pass produces (typemap/linked-javaforPublishTrimmed,typemap/javaotherwise) — no missing files (copied by_GenerateJavaStubs) and no stale files (pruned viaDeletedJavaFiles). Exercised directly by the…KeepsAndroidSrcConsistentWithLinkedJavaand…DeletesStaleAndroidSrcWhenLinkedJavaShrinkstests.5. The generator is skipped in design‑time builds and runs exactly once
_GenerateTrimmableTypeMapis gated on'$(DesignTimeBuild)' != 'true': in a design‑time build, project references may resolve to target paths that aren't produced whenSkipCompilerExecution=true, and the output isn't needed for IDE information. Combined with the existing'$(_OuterIntermediateOutputPath)' == ''guard (which skips inner per‑RID builds), the generator runs exactly once per outer build.6. Two‑pass consistency under trimming
For CoreCLR +
PublishTrimmed, correctness depends on the pre‑trim and post‑trim sentinels composing cleanly:_GenerateJavaStubskeys off the stable post‑trim stamp and is also skipped. (Newly true; previously the post‑trim pass ran every build.)_GenerateJavaStubsre‑copies..dllchange re‑runs the post‑trim pass, which touches the post‑trim stamp, so_GenerateJavaStubsre‑runs even though the pre‑trim generator did not. This is precisely the staleness the@Copilotreview flagged, and is why_GenerateJavaStubsmust key off_TrimmableJavaSourceStamprather than the generator stamp alone.What changed in this PR
…TypeMap.Trimmable.targets_GenerateTrimmableTypeMap(soleOutput, soleTouch); expandedInputs(PrivateSdkAssemblies/FrameworkAssemblies); design‑time‑build skip;SkipUnchangedFilesJCW copy; stale‑JCW deletion wiring;_GenerateJavaStubskeyed off_TrimmableJavaSourceStamp; post‑trim outputs re‑emitted in_RecordTrimmableTypeMapFileWrites.…TypeMap.Trimmable.CoreCLR.targets_ComputePostTrimTrimmableTypeMapInputs(existing‑.dllinputs) so_GeneratePostTrimTrimmableTypeMapJavaSourcesis incremental; post‑trim pass now mirrorsDeletedJavaFilesintoandroid/src+ busts the Java compile stamp.Tasks/GenerateTrimmableTypeMap.csDeletedJavaFilesoutput +DeleteStaleJavaSources(); in theCleanJavaSourceOutputDirectory(post‑trim) case it snapshots the prior JCW set before the wipe and reportsprevious − regenerated.Microsoft.Android.Sdk.TrimmableTypeMap/README.mdTrimmableTypeMapBuildTests.csandroid/src↔linked-javainvariant, post‑trim no‑op incrementality, and staleandroid/srcremoval whenlinked-javashrinks.Review feedback addressed
_GenerateJavaStubsnow keys off$(_TrimmableJavaSourceStamp)(post‑trim stamp forPublishTrimmed, generator stamp otherwise), and the non‑trim default of_TrimmableJavaSourceStampwas changed from the TypeMap DLL to the generator stamp. The underlying reason the bare generator stamp was insufficient — a non‑incremental post‑trim pass — is fixed too._GenerateTrimmableTypeMapdeclares only the stamp as itsOutputsand touches only the stamp; the DLLs/list are content‑addressed and not consumed as timestamp inputs elsewhere.Scope notes (and a little beyond)
_AndroidTypeMapImplementation == 'trimmable'; non‑trimmable (llvm-ir/managed) builds are unaffected.main's CoreCLR post‑trimlinked-javamachinery intact and instead makes it incremental.linked-javawholesale (CleanJavaSourceOutputDirectory=true) when it runs; it now also reports the JCWs it dropped soandroid/srcstays exact. A further optimization would replace the wholesale clean with content‑based writes + in‑place stale pruning (as the pre‑trim pass does), letting unchangedlinked-javaJCWs keep stable timestamps across a post‑trim run. Left as a follow‑up.Testing
Verified locally against a Debug local SDK.
Host (
Xamarin.Android.Build.Tests, fullTrimmableTypeMapBuildTests): 24 passed, 2 skipped, 0 failed, including:…IncrementalBuild(Debug CoreCLR, Release CoreCLR, Release NativeAOT)…DeletesStaleGeneratedJavaSourcesAndCopies,…CopiesUpdatedGeneratedJavaSources…PublishTrimmed_KeepsAndroidSrcConsistentWithLinkedJava(theandroid/src↔linked-javainvariant)…PublishTrimmed_DeletesStaleAndroidSrcWhenLinkedJavaShrinks(a JCW dropped fromlinked-javais removed fromandroid/src)…PublishTrimmed_PostTrimJavaGenerationIsIncremental(no‑op rebuild skips post‑trim and_GenerateJavaStubs)…Succeeds,…ArrayRankChangeRegeneratesTypeMap,…DoesNotHitCopyIfChangedMismatch, R2R/multi‑RID packaging testsEmulator (
MSBuildDeviceIntegration, API 36 / arm64):DotNetRundeploy+run for trimmable CoreCLR (Debug + Release) and NativeAOT (Release), andTrimmableTypeMapInheritedVirtualOverrideUsesCorrectUco.