Skip to content

[TrimmableTypeMap] Use proxy associations for managed -> JNI lookups - #11100

Merged
simonrozsival merged 7 commits into
mainfrom
dev/simonrozsival/trimmable-reverse-jni-lookup
Apr 10, 2026
Merged

[TrimmableTypeMap] Use proxy associations for managed -> JNI lookups#11100
simonrozsival merged 7 commits into
mainfrom
dev/simonrozsival/trimmable-reverse-jni-lookup

Conversation

@simonrozsival

@simonrozsival simonrozsival commented Apr 9, 2026 •
edited
Loading

Copy link
Copy Markdown
Member
  • emit TypeMapAssociation<Java.Lang.Object> for proxied types so the trimmable runtime has a managed -> proxy mapping
  • store the resolved JNI name on JavaPeerProxy and use it for reverse managed -> JNI lookups
  • update the trimmable runtime to consult the proxy map before attribute-based fallback, while keeping the CRC64 scanner cleanup

Copilot AI review requested due to automatic review settings April 9, 2026 10:33
@simonrozsival
simonrozsival force-pushed the dev/simonrozsival/fix-invokertype-virtual branch from 7faf5f5 to e70f93a Compare April 9, 2026 10:36
@simonrozsival simonrozsival added trimmable-type-map copilot `copilot-cli` or other AIs were used to author this labels Apr 9, 2026
Copilot AI reviewed Apr 9, 2026
View reviewed changes

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 updates the TrimmableTypeMap pipeline to support reliable managed → JNI lookups for proxied types by emitting proxy associations and persisting the resolved JNI name on generated proxy attributes, then teaching the runtime to consult this proxy mapping before falling back to attribute-based name resolution.

Changes:

  • Emit TypeMapAssociation<Java.Lang.Object> entries for all proxied types and extend generated proxy metadata to include the resolved JNI name.
  • Update runtime managed → JNI resolution to consult the proxy type map first (via TryGetJniNameForManagedType) and adjust lookup ordering accordingly.
  • Expand tests to validate CRC64 package naming for unregistered types and verify association/proxy model fields and emitted metadata filtering.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/Scanner/JavaPeerScannerTests.cs Adds a test asserting CRC64-based Java package naming for unregistered fixture types.
tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/Generator/TypeMapModelBuilderTests.cs Verifies proxies now store JniName, adds association test, and tightens PE-attribute filtering to skip association blobs.
tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/Generator/TypeMapAssemblyGeneratorTests.cs Updates assertion to handle the generic TypeMapAssociationAttribute type name.
src/Mono.Android/Microsoft.Android.Runtime/TrimmableTypeMapTypeManager.cs Switches managed → JNI lookups to consult the trimmable typemap proxy mapping first.
src/Mono.Android/Microsoft.Android.Runtime/TrimmableTypeMap.cs Adds proxy type mapping + cache behavior and a TryGetJniNameForManagedType helper that prefers proxies.
src/Mono.Android/Java.Interop/JavaPeerProxy.cs Extends proxy attribute ctor to accept and expose JniName for reverse lookups.
src/Microsoft.Android.Sdk.TrimmableTypeMap/Generator/TypeMapAssemblyEmitter.cs Updates emitted proxy ctor call to pass JniName and emits generic TypeMapAssociationAttribute ctor references.
src/Microsoft.Android.Sdk.TrimmableTypeMap/Generator/ModelBuilder.cs Populates proxy JniName and emits associations for all proxied types (aliases map to the primary proxy).
src/Microsoft.Android.Sdk.TrimmableTypeMap/Generator/Model/TypeMapAssemblyData.cs Adds required JniName field to the proxy model.

Comment thread src/Mono.Android/Java.Interop/JavaPeerProxy.cs Outdated
Comment thread tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/Scanner/JavaPeerScannerTests.cs Outdated
@simonrozsival
simonrozsival force-pushed the dev/simonrozsival/trimmable-reverse-jni-lookup branch from 1bacfe5 to 50285c7 Compare April 9, 2026 12:42
@simonrozsival simonrozsival mentioned this pull request Apr 9, 2026
Merged
Base automatically changed from dev/simonrozsival/fix-invokertype-virtual to main April 9, 2026 16:23
simonrozsival and others added 7 commits April 9, 2026 19:13
@simonrozsival
[TrimmableTypeMap] Use proxy associations for managed -> JNI lookups
beb7d7f 
Use TypeMapAssociation + JavaPeerProxy.JniName to support reverse managed-to-JNI lookups in the trimmable type map path.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@simonrozsival
[TrimmableTypeMap] Preserve final JNI names on generated proxies
6a2226e 
Thread the final emitted JNI name through generated proxy metadata and add coverage for CRC64/ACW-renamed peers so managed-to-JNI lookups resolve via the emitted name instead of compat fallbacks.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@simonrozsival
[Tests] Inline expected CRC64 Java names
6d5b4e0 
Make Scan_UnregisteredType_UsesCrc64PackageName carry the expected Java name directly in theory data so the cases are explicit and easier to review.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@simonrozsival
[TrimmableTypeMap] Remove rebased proxy regressions
9a0b1d6 
Drop the resurrected emitter getter helper, require the explicit 3-arg JavaPeerProxy constructor, and keep JNI-name fallback logic in the runtime type-map path rather than the proxy attribute base class.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@simonrozsival
[TrimmableTypeMap] Use generic JavaPeerProxy base
87c34c4 
Emit generated proxy types as JavaPeerProxy<TTarget> and call the 2-arg generic constructor so TargetType comes from the type argument rather than being redundantly passed through IL.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@simonrozsival
[Tests] Verify proxy BaseType metadata
49a500b 
Strengthen the generic proxy-base test so it inspects emitted proxy TypeDefinition.BaseType metadata and verifies the proxies really inherit from closed JavaPeerProxy<T> instantiations.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@simonrozsival
[TrimmableTypeMap] Use proxy map as the source of truth
23bd54d 
Restrict managed-type resolution in the trimmable runtime path to the generated proxy map only, and stop falling back to attribute-based JNI-name discovery when no proxy exists.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@simonrozsival
simonrozsival force-pushed the dev/simonrozsival/trimmable-reverse-jni-lookup branch from f07ad09 to 23bd54d Compare April 9, 2026 17:13
simonrozsival
simonrozsival commented Apr 9, 2026
View reviewed changes

@simonrozsival simonrozsival left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

🤖 AI Review Summary

Verdict: ⚠️ Needs Changes

Found 2 issues:

  • ❌ API design: JavaPeerProxy replaces its existing protected constructor with a new signature, which is a source/binary break for external subclasses (src/Mono.Android/Java.Interop/JavaPeerProxy.cs:21).
  • ⚠️ Patterns: TryGetJniNameForManagedType() no longer falls back to [Register] / [JniTypeSignature], so the base-type walk can miss non-proxied peers (src/Mono.Android/Microsoft.Android.Runtime/TrimmableTypeMap.cs:92).

I like the overall direction here: persisting the resolved JNI name on the proxy makes the managed → JNI path much easier to reason about. Public CI is green, but the internal Xamarin.Android-PR pipeline is still running, so this is not ready for ✅ yet.

Review generated by android-reviewer from review guidelines.

Comment thread src/Mono.Android/Java.Interop/JavaPeerProxy.cs
/// </summary>
/// <param name="targetType">The managed peer type this proxy represents.</param>
/// <param name="invokerType">The invoker type for interfaces/abstract classes, or <c>null</c> for concrete types.</param>
protected JavaPeerProxy (

@simonrozsival simonrozsival Apr 9, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

🤖 ❌ API design — JavaPeerProxy is a public type, and swapping the existing protected ctor for (string jniName, Type targetType, Type? invokerType) is a source/binary-breaking change for any downstream subclass. Please keep the old overload and chain it to the new implementation so external proxies do not have to rebuild or change signatures just to pick up this runtime fix.

Rule: Public API changes must preserve compatibility when possible.

@simonrozsival simonrozsival Apr 9, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This feature has not been released publicly so it's OK to still change the API.

Comment thread src/Mono.Android/Microsoft.Android.Runtime/TrimmableTypeMap.cs
if (type.GetCustomAttributes (typeof (IJniNameProviderAttribute), inherit: false) is [IJniNameProviderAttribute provider, ..]
&& !string.IsNullOrEmpty (provider.Name)) {
jniName = provider.Name.Replace ('.', '/');
var proxy = GetProxyForManagedType (managedType);

@simonrozsival simonrozsival Apr 9, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

🤖 ⚠️ Patterns — this helper now only succeeds when a managed type has a generated proxy association. The previous TryGetJniNameForType() also handled plain [Register] / [JniTypeSignature] attributes, and GetSimpleReferences() still walks base types expecting that fallback. Can we keep the attribute-based lookup when _proxyTypeMap misses (or add a regression test proving every relevant base type is proxy-backed)?

Rule: Verify behavior regressions when replacing an existing lookup path.

@simonrozsival simonrozsival Apr 9, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The proxy type map is generated based on the [Register] and [JniTypeSignature] attributes. It can't be missing.

@simonrozsival
simonrozsival merged commit f7080fe into main Apr 10, 2026
5 of 6 checks passed
@simonrozsival
simonrozsival deleted the dev/simonrozsival/trimmable-reverse-jni-lookup branch April 10, 2026 07:42
@simonrozsival simonrozsival mentioned this pull request Apr 10, 2026
Merged
@github-actions github-actions Bot locked and limited conversation to collaborators May 10, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

Copilot code review Copilot
Copilot review effort, defaults to Lite
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@jonathanpeppers jonathanpeppers Awaiting requested review from jonathanpeppers jonathanpeppers is a code owner

Assignees

No one assigned

Labels

copilot `copilot-cli` or other AIs were used to author this trimmable-type-map

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@simonrozsival