[TrimmableTypeMap] Emit direct RegisterNatives with UTF-8 RVA data

[simonrozsival]

Summary

Replace per-method TrimmableTypeMap.RegisterMethod() calls with a single JNI RegisterNatives call using stackalloc'd JniNativeMethod structs and compile-time UTF-8 byte data stored in RVA static fields.

What changed

• PEAssemblyBuilder: Add GetOrAddUtf8Field() for deduplicated static fields backed by raw byte data in the PE mapped field data section. Sized value types and string content are both cached — "()V" appearing in 100 proxy types is stored once.
• TypeMapAssemblyEmitter: Add type/member refs for JniNativeMethod, JniEnvironment.Types.RegisterNatives, JniType.PeerReference, and ReadOnlySpan<JniNativeMethod>. Rewrite EmitRegisterNatives to ldsflda deduplicated RVA fields for UTF-8 name/signature pointers, stackalloc JniNativeMethod[N], and call RegisterNatives once.
• TrimmableTypeMap: Remove RegisterMethod (no longer called by generated code).
• external/Java.Interop: Update submodule to c14ba04 (includes Add RegisterNatives overload for raw function pointers java-interop#1391 JniNativeMethod + raw RegisterNatives overload).

Result

Zero delegate allocations, zero string allocations, zero array allocations, one JNI call per class during native registration.

Dependencies

• Add RegisterNatives overload for raw function pointers java-interop#1391 (already in submodule)
• Base: [TrimmableTypeMap] Implement runtime TypeManager, ValueManager, and JavaConvert #10967

[Copilot]

Pull request overview

This PR updates the generated ACW registration codepath to use a single JniEnvironment.Types.RegisterNatives call with stackalloc’d JniNativeMethod entries and deduplicated UTF-8 data stored in PE RVA field data, removing the older per-method TrimmableTypeMap.RegisterMethod() helper.

Changes:

• Emit RegisterNatives once per type using JniNativeMethod + ReadOnlySpan<JniNativeMethod> and method pointers.
• Add PEAssemblyBuilder.GetOrAddUtf8Field() with mapped field data + caching to deduplicate UTF-8 literals.
• Remove TrimmableTypeMap.RegisterMethod() and add generator tests validating new emission behavior.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/Generator/TypeMapAssemblyGeneratorTests.cs	Adds tests ensuring direct RegisterNatives usage, presence of <PrivateImplementationDetails>, and UTF-8 field dedup behavior.
src/Mono.Android/Microsoft.Android.Runtime/TrimmableTypeMap.cs	Removes the obsolete per-method RegisterMethod helper.
src/Microsoft.Android.Sdk.TrimmableTypeMap/Generator/TypeMapAssemblyEmitter.cs	Reworks emitted IL to build JniNativeMethod arrays via stackalloc and call RegisterNatives once.
src/Microsoft.Android.Sdk.TrimmableTypeMap/Generator/PEAssemblyBuilder.cs	Adds mapped field data support and deduped UTF-8 RVA field creation.
external/Java.Interop	Updates submodule to include JniNativeMethod and raw RegisterNatives overload.

[Copilot]

JniNativeMethod initialization IL is very likely invalid due to stack type mismatches: (1) ldsflda pushes a managed byref to the RVA field’s value-type, not a byte*; passing it to a byte* parameter usually requires an explicit conversion (e.g., conv.u) or otherwise producing an unmanaged pointer. (2) Calling a value-type instance .ctor with call expects a managed byref (&JniNativeMethod) as the this argument; here methods[i] is computed from localloc as a native integer pointer, which generally won’t satisfy the required & type. A more robust pattern is to construct the struct value with newobj and then store it into methods[i] with stobj (or otherwise follow the IL pattern produced by the C# compiler for stackalloc + element assignment). This should prevent BadImageFormatException / invalid IL at runtime.

[Copilot]

RegisterNatives(JniType jniType) passes jniType by value/reference depending on whether JniType is a reference type (it commonly is). Using LoadArgumentAddress(1) will push an argument-slot address (&), which is not a valid receiver for an instance property getter on a reference type and can produce invalid IL at runtime. Use LoadArgument(1) (and consider callvirt if the target is a reference type and you want the normal null-check semantics).

Suggested change

	encoder.LoadArgumentAddress (1);
	encoder.LoadArgument (1);

[Copilot]

ReadOnlySpan type refs are being re-added in multiple places (here, plus EmitTypeReferences() and EncodeReadOnlySpanOfJniNativeMethod()), which can create duplicate TypeRef rows and makes handle usage harder to reason about. Prefer caching a single TypeReferenceHandle (e.g., _readOnlySpanRef) and reusing it consistently in the member refs, local signature encoding, and parameter signature encoding.

[Copilot]

Field RVA data alignment can matter for PE/CLR readers. Writing raw bytes back-to-back and using the current _mappedFieldData.Count as the next RVA risks generating misaligned RVAs depending on platform/reader expectations. Consider aligning _mappedFieldData to at least 4 bytes (or the required alignment for mapped field data) before recording rva and writing each payload, padding with zeros as needed.

[Copilot]

The comment claims the RVA field lives on <PrivateImplementationDetails>, but GetOrAddUtf8Field() calls GetOrCreateSizedType() (which adds a new nested TypeDefinition) immediately before AddFieldDefinition(). In ECMA-335 metadata, fields are associated to the most recently added TypeDefinition by table ordering, so these fields will typically end up belonging to the last created __utf8_{size} nested type, not <PrivateImplementationDetails>. Either update the documentation to match the actual metadata layout, or refactor emission so <PrivateImplementationDetails> remains the declaring type for these fields (e.g., avoid creating additional TypeDefinition rows between creating <PrivateImplementationDetails> and adding its fields, potentially by precomputing/creating sized types earlier or making sized types non-nested/top-level).

Suggested change

	/// The field lives on a shared <c>&lt;PrivateImplementationDetails&gt;</c> type.
	/// </summary>
	/// The field is declared on an internal sized helper type (typically nested under
	/// <c>&lt;PrivateImplementationDetails&gt;</c>) and is considered an implementation detail.

[Copilot]

ownerType is introduced but not used inside this method in the shown changes. If it’s not needed, remove it to avoid confusion; if it’s intended for future use (e.g., to scope UTF-8 fields or member refs), consider wiring it in now or adding a short comment explaining the planned usage.

Suggested change

	{
	{
	// ownerType is reserved for future use (e.g., to scope UTF-8 fields or member refs).
	_ = ownerType;

[simonrozsival]

🤖 AI Review Summary

Verdict: ⚠️ Needs Changes

Found 3 issues (0 error, 2 warning, 1 suggestion):

• ⚠️ Performance: ReadOnlySpan\1type reference is re-created inencodeLocalsevery timeEmitRegisterNatives is called (TypeMapAssemblyEmitter.cs:857`)
• ⚠️ Documentation: Doc comment example still shows packed-blob offset pattern but implementation uses per-string fields (TypeMapAssemblyEmitter.cs:52)
• 💡 Code organization: ownerType parameter is unused after dedup refactor (TypeMapAssemblyEmitter.cs:751)

👍 Excellent change overall. The RVA field approach eliminates all allocations during native method registration — no delegates, no strings, no arrays. String deduplication across proxy types is a nice touch. The <PrivateImplementationDetails> pattern mirrors what the C# compiler does. Good test coverage with 3 new tests.

---

Review generated by android-reviewer from review guidelines.

[simonrozsival]

🤖 ⚠️ Performance — _pe.Metadata.AddTypeReference for ReadOnlySpan\1is called inside theencodeLocalscallback, which runs every timeEmitRegisterNatives` is invoked (once per ACW proxy type). This creates a duplicate type reference row in the metadata table for each proxy.

Consider moving this to EmitTypeReferences() where readOnlySpanRef is already created for the TypeSpec, and reuse the cached handle here.

Rule: Cache repeated accessor calls / Avoid unnecessary allocations

[simonrozsival]

🤖 ⚠️ Documentation — This doc comment example still shows the packed-blob offset pattern (__utf8_natives[0], __utf8_natives[11]) but the implementation now uses per-string deduplicated fields with ldsflda. Should be updated to reflect the actual generated code, e.g.:

///     methods[0] = new JniNativeMethod(&__utf8_0, &__utf8_1, &n_OnCreate_uco_0);
///     methods[1] = new JniNativeMethod(&__utf8_2, &__utf8_1, &nctor_0_uco);

Rule: Remove stale comments (Postmortem #59)

[simonrozsival]

🤖 💡 Code organization — The ownerType parameter is no longer used after the refactor to deduplicated fields on <PrivateImplementationDetails>. Consider removing it from the signature and the callsite at line 415.

Rule: Remove unused code (Postmortem #58)