Repository navigation
[TrimmableTypeMap] Avoid assembly reflection during peer lookup - #13526
simonrozsival wants to merge 4 commits into
Conversation
NativeAOT repeatedly constructs reflection assembly metadata while checking whether a sealed target belongs to Mono.Android. Move that eligibility check into typemap generation and encode it in a marker proxy base class, without adding state to each proxy instance. Keep external peers on the Java hierarchy lookup path because they may use a different class loader. Include the generated base in typemap fingerprints and cover aliases, generic targets, and runtime fallback. Fixes #13421. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
The alias test cases emit no proxies, so their assertions never run and base-class and constructor selection remain untested.
1 open issue
What changed in this PR
Addresses #13421 by moving sealed-framework peer eligibility checks from runtime reflection into generated proxy types. This reduces NativeAOT peer-lookup overhead while preserving hierarchy lookup for external peers that may use different class loaders.
Changes:
- Adds a marker base for sealed
Mono.Androidpeers and uses it during runtime lookup. - Propagates eligibility through scanning, generation, constructor emission, and both typemap fingerprints.
- Adds regression tests and updates the public API baseline and interop documentation.
| File | Description |
|---|---|
tests/Mono.Android-Tests/Mono.Android-Tests/Java.Interop/TrimmableTypeMapTypeManagerTests.cs |
Extends device tests for marker eligibility and hierarchy lookup. |
src/Mono.Android/PublicAPI/API-37/PublicAPI.Unshipped.txt |
Records the new marker base API. |
src/Mono.Android/Microsoft.Android.Runtime/TrimmableTypeMap.cs |
Replaces reflection-based eligibility checks with marker detection. |
src/Mono.Android/Java.Interop/JavaPeerProxy.cs |
Defines the marker base without additional instance fields. |
src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/JavaPeerScanner.cs |
Captures sealed-type metadata. |
src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/JavaPeerInfo.cs |
Stores sealed status in scanner results. |
src/Microsoft.Android.Sdk.TrimmableTypeMap/Generator/TypeMapAssemblyEmitter.cs |
Emits the selected base and matching constructor reference. |
src/Microsoft.Android.Sdk.TrimmableTypeMap/Generator/ModelBuilder.cs |
Restricts marker eligibility to sealed Mono.Android types. |
src/Microsoft.Android.Sdk.TrimmableTypeMap/Generator/Model/TypeMapAssemblyData.cs |
Carries eligibility in the generation model. |
src/Microsoft.Android.Sdk.TrimmableTypeMap/Generator/MetadataHelper.cs |
Includes eligibility in both fingerprints. |
src/Microsoft.Android.Build.Tasks/Tests/Microsoft.Android.Build.Tasks.Tests/TrimmableTypeMapIncrementalTests.cs |
Tests invalidation and deterministic identity changes. |
src/Microsoft.Android.Build.Tasks/Tests/Microsoft.Android.Build.Tasks.Tests/GenerateTrimmableTypeMapTests.cs |
Adds parameterized base-class and constructor checks. |
Documentation/guides/internals/JavaJNI_Interop.md |
Explains marker-based lookup and class-loader restrictions. |
Review effort: Balanced
|
/review |
|
✅ Android PR Reviewer completed successfully!
|
The sealed-peer generator test used alias peers without activation constructor metadata. Alias groups therefore emitted no proxies, making the loop over generated proxy types vacuous for those cases. Give both peers activation metadata and assert that the expected proxies are emitted before checking their base and constructor references. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Findings: 0 ❌ errors · 0
The scanner-to-model-to-emitter path consistently limits the marker base to sealed Mono.Android targets, and the emitted constructor calls the selected base. Including eligibility in both fingerprints correctly covers incremental invalidation and deterministic MVID changes. The runtime retains the existing remapping, assignability check, and JNI-reference cleanup. The latest test-only commit c315011017 also fixes the previously vacuous alias cases by supplying activation metadata and asserting the proxy count.
CI: GitHub reports five Azure DevOps checks in progress, the aggregate check queued, and CLA passing for the current head. Azure build 1678182 is not confirmed green; the direct Azure build query requires authentication, so deeper pipeline status was unavailable. The PR's reported local test and benchmark results were not independently rerun during this review.
Generated by Android PR Reviewer for #13526 · copilot · gpt61 · 63.6 AIC · ⌖ 7.58 AIC · ⊞ 19.5K
Comment /review to run again
Context: #13526 The existing external-peer test created its Java object through the runtime loader, so it did not cover duplicate class identity across loaders. Load the fixture from the app APK using a PathClassLoader with a null parent, assert the object is not an instance of the runtime-loader class, and verify typemap lookup still chooses the unmarked external proxy. The Release on-device runtime suite passed: 1,057 passed, 8 skipped. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
CA1422 fails the build because `Class.NewInstance()` is obsoleted on API-34+. Use `Class.GetConstructor().NewInstance()` instead. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
/review |
|
✅ Android PR Reviewer completed successfully!
|
There was a problem hiding this comment.
Code assessment: no blocking findings. 0 ❌ errors, 0
The scanner-to-model-to-emitter change consistently selects the marker base and its constructor only for sealed Mono.Android targets. The runtime retains the class-loader safeguard for external peers, and the eligibility bit participates in both incremental and content fingerprints. The generated-base/constructor matrix, incremental invalidation checks, and on-device class-loader and generic cases provide useful coverage. The documented NativeAOT measurements support the narrowly stated performance claim; the description appropriately avoids claiming a demonstrated CoreCLR or whole-app speedup.
CI verification is incomplete: all 40 GitHub checks report success, including the aggregate dotnet-android check for Azure build 1679658. Direct Azure build/timeline verification was blocked: the build query required authentication, and the timeline query failed during CLI initialization. I therefore cannot independently confirm the Azure result or per-stage evidence. This was a static review of head d758a06740f800b81fc2e055fa0c51465bcff580; host/device tests and benchmarks were not rerun.
Generated by Android PR Reviewer for #13526 · copilot · gpt61 · 59.1 AIC · ⌖ 4.74 AIC · ⊞ 20.2K
Comment /review to run again
|
|
||
| var targetProxy = GetProxyForManagedType (targetType); | ||
| if (targetProxy is null) { | ||
| GetProxyForManagedType (targetType) is not JavaPeerProxyForSealedFrameworkClass targetProxy) { |
There was a problem hiding this comment.
🤖 💡 Performance — Could you add a nonsealed target, such as Java.Lang.Object, to the before/after lookup measurements? Previously !targetType.IsSealed returned before GetProxyForManagedType; now every non-null nonsealed target also performs the managed-proxy lookup (including generic normalization and, on a cold cache, attribute instantiation) before falling back to hierarchy traversal. The sealed framework/app measurements establish the intended improvement, but do not cover this additional work on the nonsealed path. A nonsealed case would help establish that the optimization does not trade that improvement for a regression in common base-type lookups.
{Rule: Consider startup-time impact; place cheap checks before expensive ones}
| /// <summary> | ||
| /// Base class for generated proxies whose sealed Mono.Android target can be resolved | ||
| /// using the runtime's class loader without inspecting the Java object's hierarchy. | ||
| /// </summary> | ||
| [EditorBrowsable (EditorBrowsableState.Never)] | ||
| public abstract class JavaPeerProxyForSealedFrameworkClass : JavaPeerProxy |
There was a problem hiding this comment.
Do we need a new class? (this one has an odd name) Or could we make a new JavaPeerProxy.IsSealed property?
Fixes #13421.
Context: #12644.
The sealed-peer shortcut checks
Type.Assemblyon every lookup. NativeAOT resolves this through runtime metadata, making the check disproportionately expensive. Replace it with a generator-selectedJavaPeerProxyForSealedFrameworkClassbase for sealed types defined inMono.Android; the runtime recognizes that marker without adding a field to each proxy.The
Mono.Androidrestriction preserves the existing class-loader safeguard.FindClasscan resolve an app class using a loader different from the one that created the Java object; classes with the same name from different loaders have different identities. AnIsInstanceOfmismatch can therefore be inconclusive for external peers, which continue to use hierarchy lookup. Android normally uses separate framework and app loaders, and additional loaders can be created at runtime, so build inputs cannot generally establish which class identities will be involved in a lookup.CanResolveFromSealedTargetTypeis generator-model metadata, not a second runtime marker. It selects the emitted proxy base and is included in both the incremental-build fingerprint and the deterministic MVID content fingerprint: changing a peer's eligibility changes the emitted assembly even when its type name is unchanged. This prevents a stale incremental output and gives different emitted content a different MVID.Cover generated bases and constructors across framework/external, sealed/unsealed, alias, and generic shapes; incremental invalidation and deterministic MVIDs; and on-device sealed success/mismatch, nonsealed hierarchy lookup, external peers, and closed generics.
NativeAOT measurements
A fully trimmed Release
android-arm64instrumentation app repeatedly creates managed peers usingJava.Lang.Object.GetObject<T>from retained JNI references, disposing the wrappers between batches. Android 15 ARM64 emulator on Apple Silicon; baselinea008bb3473bfa512f9f6ce5823ea2bf01d82981fversus this change, same app source and build settings. Each case uses 20 warmup batches and 80 measured batches of 4,096 lookups per process. Six interleaved installs/runs (before, after, after, before, before, after), without profiling during timing: 240 measured batches per case/variant. GC and disposal are outside the timed regions.Java.Lang.StringPer-run medians do not overlap: framework 4,879–5,046 ns before versus 1,955–2,051 ns after; app peer 4,016–4,234 ns versus 2,401–2,531 ns. Separate 4 kHz
cpu-clocksimpleperf captures with native symbols showNativeFormatRuntimeNamedTypeInfo.get_Assemblyat 34.29% inclusive samples before and no sampled calls after. Both APKs contain the NativeAOT library, and instrumentation asserts that dynamic code is unsupported.CoreCLR measurements
The same app and workload, rebuilt with
PublishAot=falseandAndroidRuntime=CoreCLR, ran twice with the same six-run interleaving above: six processes and 480 measured batches per case/variant in total. The APKs containlibcoreclr.so; instrumentation verifies dynamic-code support and the trimmable typemap value manager.Java.Lang.StringThe ranges overlap substantially, so these CoreCLR results do not establish a measurable speedup. The first before run was especially variable; the second round's pooled framework medians were 1,997 ns before and 1,935 ns after.
For this CoreCLR baseline,
make prepare CONFIGURATION=Releasepassed. A cleanIncludeAndroidJavadoc=True make leeroy CONFIGURATION=Releasefailed in Javadoc generation due to missing copyright metadata.IncludeAndroidJavadoc=False make leeroy CONFIGURATION=Releasebuilt the baseline binaries and extra API levels but failed packaging becauseMono.Android.xmlwas absent. Copying that documentation XML from the changed SDK and runningIncludeAndroidJavadoc=False ./bin/Release/dotnet/dotnet build -p:Configuration=Release -t:ConfigureLocalWorkload build-tools/create-packs/Microsoft.Android.Sdk.projcompleted packaging. Thus the baseline used its own runtime and generator binaries but differs in documentation-generation settings; the copied XML is packaging input, not benchmark code.These are peer-lookup microbenchmark results, not PoolMath, MAUI scrolling, physical-device, or whole-app speedup claims.
Local validation
make prepare CONFIGURATION=Release— passed.IncludeAndroidJavadoc=True make leeroy CONFIGURATION=Release— passed before and after, including extra API levels and local workload configuration. The initial invocation without documentation enabled failed because packaging requiredMono.Android.xml; enabling the existing documentation option resolved it without build-file changes.On-device commands: NativeAOT 20 passed; CoreCLR 20 passed
Both configurations built, installed, and passed all 20 selected tests on the ARM64 emulator. Existing SourceLink and trim/AOT-analysis warnings remain visible.