[xabt] Move trimmable typemap tasks to net11 - #12678
Conversation
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
It changes shipped MSBuild task loading/target wiring and retargets generator infrastructure, which warrants careful human validation of build/packaging behavior across MSBuild runtimes.
Review tier: Lite
Findings: None
What changed in this PR
This PR restructures the trimmable typemap pipeline by moving the MSBuild task entrypoints into the Microsoft.Android.Build.Tasks net11 assembly, retargeting the generator/tests to .NET 11, and updating the SDK targets to load the new tasks appropriately under both Full Framework and Core MSBuild.
Changes:
- Rewired
Microsoft.Android.Sdk.TypeMap.Trimmable*.targetsto loadMicrosoft.Android.Tasks.GenerateTrimmableTypeMap/GenerateMissingTypeMapStubsfromMicrosoft.Android.Build.Tasks.dll. - Retargeted
Microsoft.Android.Sdk.TrimmableTypeMap(and its unit tests) to$(DotNetTargetFramework)and adjusted code for .NET 11 runtime behavior. - Added a dedicated
Microsoft.Android.Build.Tasks.Testsproject and migrated relevant task tests, while keeping unrelated NativeAOT ProGuard tests in the existing suite.
| File | Description |
|---|---|
| Xamarin.Android.slnx | Adds the new Microsoft.Android.Build.Tasks.Tests project to the main solution test folder. |
| Xamarin.Android.Build.Tasks.slnx | Adds the new Microsoft.Android.Build.Tasks.Tests project to the build-tasks solution. |
| tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests.csproj | Retargets unit tests to $(DotNetTargetFramework) for the net11 transition. |
| src/Xamarin.Android.Build.Tasks/Xamarin.Android.Build.Tasks.csproj | Adds explicit System.Reflection.Metadata package reference and removes the trimmable typemap project reference. |
| src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Tasks/GenerateNativeAotProguardConfigurationTests.cs | Introduces a dedicated test file for NativeAOT ProGuard configuration behavior after test relocation. |
| src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.TypeMap.Trimmable.targets | Updates <UsingTask> to load GenerateTrimmableTypeMap from Microsoft.Android.Build.Tasks. |
| src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.TypeMap.Trimmable.CoreCLR.targets | Updates <UsingTask> to load GenerateMissingTypeMapStubs from Microsoft.Android.Build.Tasks. |
| src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.targets | Adds _MicrosoftAndroidBuildTasksAssembly path for the new task assembly. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/ScannerHashingHelper.cs | Adjusts CRC64 hashing implementation to use compatible APIs in the presence of MSBuild’s asset loading behavior. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/JavaPeerScanner.cs | Tightens forwarded-type resolution with an explicit null-aware lookup. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/JavaAnnotationParser.cs | Tightens dictionary lookup/null handling around resolved indices. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/README.md | Updates documentation paths and notes net11 generator/task location. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Microsoft.Android.Sdk.TrimmableTypeMap.csproj | Retargets generator library to $(DotNetTargetFramework) and removes now-unneeded references for net11. |
| src/Microsoft.Android.Build.Tasks/Tests/Microsoft.Android.Build.Tasks.Tests/Microsoft.Android.Build.Tasks.Tests.csproj | Adds a new net11 test project for Microsoft.Android.Build.Tasks. |
| src/Microsoft.Android.Build.Tasks/Tests/Microsoft.Android.Build.Tasks.Tests/GenerateTrimmableTypeMapTests.cs | Updates tests to use the moved task type and adjusts Mono.Android discovery logic. |
| src/Microsoft.Android.Build.Tasks/Tests/Microsoft.Android.Build.Tasks.Tests/GenerateMissingTypeMapStubsTests.cs | Updates tests to reference Microsoft.Android.Tasks task types. |
| src/Microsoft.Android.Build.Tasks/Tests/Microsoft.Android.Build.Tasks.Tests/BaseTest.cs | Adds a lightweight base test helper for the new task test project. |
| src/Microsoft.Android.Build.Tasks/Tasks/GenerateTrimmableTypeMap.cs | Moves task entrypoint into Microsoft.Android.Tasks and adjusts helper implementations. |
| src/Microsoft.Android.Build.Tasks/Tasks/GenerateMissingTypeMapStubs.cs | Moves task entrypoint namespace to Microsoft.Android.Tasks. |
| src/Microsoft.Android.Build.Tasks/Microsoft.Android.Build.Tasks.csproj | Adds direct references needed for moved tasks and links shared utilities/resources. |
| .github/skills/tests/SKILL.md | Updates the tests skill guidance to include the new standalone task tests. |
| .github/skills/tests/references/test-catalog.md | Adds the new Microsoft.Android.Build.Tasks.Tests project to the test catalog. |
Suppressed comments (1)
src/Microsoft.Android.Build.Tasks/Tasks/GenerateTrimmableTypeMap.cs:389
⚠️ warning Error handling —LoadCustomViewTypeNames()assumes every line contains a;separator and will throwIndexOutOfRangeExceptionon blank/malformed lines (e.g., an empty trailing line or manual edits), failing the task. Consider parsing defensively and only collecting the keys since the values aren’t used.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
/review |
|
✅ Android PR Reviewer completed successfully!
|
There was a problem hiding this comment.
✅ LGTM
No correctness or safety issues found. 0 errors · 0 warnings · 1 suggestion. The task migration keeps modern dependencies isolated under tools/net, updates both Core and full-framework registrations consistently, preserves localized resources, and adds focused task/resource coverage. All 44 CI checks passed.
The inline suggestion is non-blocking and asks for explicit coverage of the full-framework TaskHostFactory execution path.
Generated by Android PR Reviewer for #12678 · gpt56 · 328.4 AIC · ⌖ 20.6 AIC · ⊞ 25.7K
Comment /review to run again
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
jonathanpeppers
left a comment
There was a problem hiding this comment.
There is a merge conflict
…le-typemap-migration
## Summary - move the two JNI assembly rewriter tests that directly instantiate `Microsoft.Android.Sdk.TrimmableTypeMap` types from the `net10.0` `Xamarin.Android.Build.Tests` assembly to the existing `net11.0` trimmable type map integration test assembly - preserve the assertions that no `XA4326` warning is emitted - restore compilation after #12678 removed the transitive type map project reference from `Xamarin.Android.Build.Tasks` This fixes the cross-platform compilation regression exposed by Azure DevOps build 1587962 without adding a `net11.0` project reference to the `net10.0` test project. ## Validation - `.\dotnet-local.cmd build src\Xamarin.Android.Build.Tasks\Tests\Xamarin.Android.Build.Tests\Xamarin.Android.Build.Tests.csproj -v:minimal` - `.\dotnet-local.cmd test tests\Microsoft.Android.Sdk.TrimmableTypeMap.IntegrationTests\Microsoft.Android.Sdk.TrimmableTypeMap.IntegrationTests.csproj -v:minimal --filter "FullyQualifiedName~JniAssemblyRewriterTests"` (2 passed)
Follow-up to #12678, using the generator's new .NET 11 target to reduce temporary allocations. Five focused optimizations: - Validate Java names with spans, alternate keyword lookups, and span NFC checks. - Construct proxy, alias, and Java names with `string.Create` and span replacements. - Replace scanner `params` arrays with `ReadOnlySpan<string>`. - Parse connector and assembly-qualified names with spans and use alternate type-name lookups. - Use span-based UTF-8 encoding and `SHA256.HashData` for deterministic metadata hashing. Preserves Java 21 identifier rules, generated names, resolution order, and hash serialization. Larger structural optimizations are left for follow-up PRs. Review follow-up: move `GetAndroidPackageName` and its existing coverage into the net11.0 task project, remove the validator/identifier-data source links from the netstandard2.0 assembly, and package the task's Android SDK helper dependency. Validation results now remain spans until the diagnostic boundary; no compatibility implementation of the validator is retained. ### Allocation measurements Compared `386608ea08` (before these optimizations) with `8026fb78ec`, using identically configured Release generator builds and the same probe binary on macOS Arm64, .NET 11 preview 7, workstation GC. These measurements precede the package-task migration and span-output follow-up. The workload uses the installed Android API 37 preview 7 reference assemblies plus a small app: 9,087 peers, 5,759 marshal methods, and 332 generated Java sources. Full generation emits four typemap assemblies totaling 5,406,720 bytes. It uses per-assembly typemap universes and `collectMarshalMethodsForNonAcw=false`, matching the Debug task path. The fingerprint-hit case still scans, models, fingerprints, and generates Java/manifest content, but its callback skips typemap DLL emission; it is not an MSBuild no-op build. Five alternating baseline/current samples per case, each with 10 warm-up invocations and 30 measured invocations. Values below are medians; MB is decimal. | Path | Allocated bytes/invocation before | After | Reduction | Gen 0 collections per 30 invocations | | --- | ---: | ---: | ---: | ---: | | Full generation | 135.21 MB | 128.13 MB | **5.23%** | 373 → 351 | | Fingerprint hit / skip DLL emission | 87.14 MB | 80.25 MB | **7.91%** | 277 → 253 | Allocation counters use `GC.GetTotalAllocatedBytes(precise: true)` around the measured loop; startup, input-file reading, and warm-up are excluded. Separate `dotnet-trace collect --profile gc-verbose` captures corroborate the reduction, with measurement-window markers and zero lost events in all four traces. Sampled allocations show fewer temporary strings, `char[]`, `string[]`, and `StringBuilder` objects. Generated DLL/Java/manifest checksums match between variants in each case. This establishes lower managed allocation volume and fewer Gen 0 collections, not a general latency improvement. Gen 1/2 counts were roughly flat or slightly higher. Median total GC pause time per sample was 1,098.6 → 1,006.2 ms for full generation and 839.3 → 843.0 ms for the fingerprint-hit case. Untraced median time per invocation was 131.1 → 131.2 ms and 99.0 → 108.0 ms respectively; ranges overlapped, and traced timings changed the ordering. No end-to-end build-speed claim is made. This profiles the standalone net11.0 generator, not end-to-end MSBuild execution.
Summary
GenerateTrimmableTypeMapandGenerateMissingTypeMapStubsinto the net11.0Microsoft.Android.Build.Tasksassemblytools/net/, avoiding conflicts with netstandard2.0 dependencies in the shared tools directorySystem.Reflection.Metadatausage explicitPerformance improvements remain intentionally out of scope; existing stack-based hashing continues to use
System.IO.Hashing.Crc64directly.Validation
make prepare && make allBuild_WithTrimmableTypeMap_Succeedscases