[TrimmableTypeMap] Fix trimmable Java object activation#11275
Open
simonrozsival wants to merge 6 commits into
Open
[TrimmableTypeMap] Fix trimmable Java object activation#11275simonrozsival wants to merge 6 commits into
simonrozsival wants to merge 6 commits into
Conversation
711aeae to
7e2737b
Compare
a8491be to
3bcb461
Compare
7e2737b to
dc13b42
Compare
3bec3d2 to
ddcddd4
Compare
ddcddd4 to
daeb39a
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the TrimmableTypeMap pipeline to improve Java→managed activation behavior (notably around constructor callbacks), marks certain activation peers as replaceable to support Java-side virtual dispatch scenarios, and re-enables previously excluded device tests as those behaviors are restored.
Changes:
- Updates typemap proxy/UCO emission to avoid reflection for default managed constructors and to mark activation peers as
Replaceable. - Extends the typemap scanner/model to treat
JniAddNativeMethodRegistrationAttributetypes (hand-written Java peers) as requiring ACW/proxy support. - Adjusts test infrastructure and test inputs (C#/Java) to run trimmable-specific Java.Interop constructor-virtual-call scenarios and to remove now-unneeded exclusions.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/Mono.Android-Tests/Mono.Android-Tests/Xamarin.Android.RuntimeTests/NUnitInstrumentation.cs | Removes trimmable exclusions for tests that are now expected to pass. |
| tests/Mono.Android-Tests/Mono.Android-Tests/Java.Lang/ObjectTest.cs | Removes an outdated TODO tied to trimmable typemap behavior. |
| tests/Mono.Android-Tests/Mono.Android-Tests/Java.Interop/JnienvTest.cs | Cleans up throwable activation test logging; adds TODO for open generic activation behavior. |
| tests/Mono.Android-Tests/Java.Interop-Tests/Java.Interop-trimmable/CallVirtualFromConstructorDerived.cs | Adds a trimmable-specific managed peer used for constructor virtual-dispatch testing. |
| tests/Mono.Android-Tests/Java.Interop-Tests/Java.Interop-Tests.targets | Adjusts Java test-jar inputs so trimmable builds use java-trimmable sources for specific classes. |
| tests/Mono.Android-Tests/Java.Interop-Tests/Java.Interop-Tests.NET.csproj | Conditionally swaps managed CallVirtualFromConstructorDerived source between desktop and trimmable variants. |
| tests/Mono.Android-Tests/Java.Interop-Tests/java-trimmable/net/dot/jni/test/CallVirtualFromConstructorDerived.java | Adds trimmable Java peer for derived constructor-virtual-call scenario. |
| tests/Mono.Android-Tests/Java.Interop-Tests/java-trimmable/net/dot/jni/test/CallVirtualFromConstructorBase.java | Adds trimmable Java peer base class for the same scenario. |
| src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/TrimmableTypeMapBuildTests.cs | Expands SDK pack assertions to ensure obsolete java-runtime artifacts aren’t shipped. |
| src/Mono.Android/Java.Interop/JavaPeerProxy.cs | Adds helper to mark activation peers as Replaceable. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/JavaPeerScanner.cs | Records presence of JniAddNativeMethodRegistrationAttribute for downstream model decisions. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/JavaPeerInfo.cs | Adds model flag for JniAddNativeMethodRegistrationAttribute usage. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Generator/TypeMapAssemblyEmitter.cs | Changes proxy/UCO emission logic (default ctor activation path + replaceable marking; also refactors typemap attribute emission). |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Generator/ModelBuilder.cs | Ensures handwritten Java peers with native registrations can trigger proxy/ACW generation. |
| src/java-runtime/java-runtime.targets | Deletes/cleans obsolete java-runtime artifacts to prevent stale outputs and packaging. |
Comments suppressed due to low confidence (3)
src/Microsoft.Android.Sdk.TrimmableTypeMap/Generator/TypeMapAssemblyEmitter.cs:184
EmitCore()no longer emits per-rank__ArrayMapRank{N}sentinel TypeDefs (previously viaEmitRankSentinels(model)), butModelBuildercan still produce array entries (TypeMapAttributeData.AnchorRank/model.MaxArrayRank) and the root typemap loader expects those anchors to exist for array lookups. This will breakTrimmableTypeMap.Instance.TryGetArrayType(...)/JNIEnvarray creation when_AndroidTrimmableTypeMapMaxArrayRank> 0. Consider restoring rank-sentinel emission (and the corresponding per-rank TypeMap grouping) or removing/rewriting the array-entry pipeline end-to-end so the root loader and runtime agree.
throw new ArgumentNullException (nameof (model));
}
if (stream is null) {
throw new ArgumentNullException (nameof (stream));
}
EmitCore (model, useSharedTypemapUniverse);
_pe.WritePE (stream);
}
void EmitCore (TypeMapAssemblyData model, bool useSharedTypemapUniverse)
{
_pe.EmitPreamble (model.AssemblyName, model.ModuleName, MetadataHelper.ComputeContentFingerprint (model));
_javaInteropRef = _pe.AddAssemblyRef ("Java.Interop", new Version (0, 0, 0, 0));
EmitTypeReferences ();
if (useSharedTypemapUniverse) {
src/Microsoft.Android.Sdk.TrimmableTypeMap/Generator/TypeMapAssemblyEmitter.cs:1402
EmitTypeMapAttribute()now ignoresentry.AnchorRankand always uses the default anchor’sTypeMapAttribute<TGroup>ctor. Any array entries created withAnchorRank(seeTypeMapAttributeData.AnchorRank) will end up in the wrong typemap universe/group, so the root loader’s per-rankTypeMapping.GetOrCreateExternalTypeMapping<__ArrayMapRankN>()maps won’t contain them. Either reintroduce the per-rank anchor handling here, or stop generatingAnchorRankentries (and update the runtime/root loader accordingly).
encoder.Token (nameFields [i]);
// byte* signature
encoder.OpCode (ILOpCode.Ldsflda);
encoder.Token (sigFields [i]);
// IntPtr functionPointer
encoder.OpCode (ILOpCode.Ldftn);
encoder.Token (validRegs [i].Wrapper);
// Construct the struct on the evaluation stack and store it
// at the destination address. This matches the Roslyn pattern:
// newobj JniNativeMethod::.ctor(byte*, byte*, IntPtr)
// stobj JniNativeMethod
src/Microsoft.Android.Sdk.TrimmableTypeMap/Generator/TypeMapAssemblyEmitter.cs:1008
- For
proxy.IsGenericDefinition,EmitUcoConstructor()now emits a no-op wrapper that silently returns. This changes behavior from “fail activation” to “succeed without a managed peer”, which can leave an allocated Java instance without a corresponding managed wrapper and also conflicts with existing expectations that open generic activation is unsupported (e.g.,JnienvTest.NewOpenGenericTypeThrows). It would be safer to preserve the previous behavior of throwing aNotSupportedException(or otherwise surfacing an error) so the failure is explicit and debuggable.
encoder.Call (_waitForBridgeProcessingRef);
encoder.MarkLabel (tryStart);
emitCallback (encoder);
if (!isVoid) {
encoder.StoreLocal (0);
}
encoder.Branch (ILOpCode.Leave, afterAll);
encoder.MarkLabel (catchStart);
encoder.StoreLocal (isVoid ? 0 : 1);
50e081e to
02e0cf4
Compare
Run default managed constructors for generated no-arg constructor callbacks, and mark generated constructor activation peers as replaceable so managed wrappers can replace temporary peers created during Java-side virtual dispatch. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Avoid reflection for generated no-arg Java constructor callbacks by emitting IL that attaches the JNI peer and invokes the managed default constructor directly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
After rebasing onto main (which merged #11260 adding maxstack tracking), the new 0-arg nctor codegen path in EmitUcoMethod was using the pre-#11260 bare OpCode + Token style. Switch to the tracked encoder helpers (LoadToken, CastClass, Call(parameterCount,…), Callvirt(parameterCount,…)) so the emitted method's maxstack is computed correctly, matching every other IL site in TypeMapAssemblyEmitter. Also update Generate_InheritedCtor_* / Generate_InheritedJavaInteropCtor_* generator tests to reflect the new nctor contract: the default-ctor branch references the target type's parameterless .ctor(), SetPeerReference and MarkActivationPeerReplaceable — and no longer references the legacy (IntPtr, JniHandleOwnership) / (JniObjectReference&, JniObjectReferenceOptions) activation ctor on the inherited base. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
02e0cf4 to
b22aef2
Compare
Add broad constructor activation coverage and generate direct constructor UCO activation for default and parameterized Java constructors. The generated path now reuses activation peers, skips managed construction recursion, forwards JNI constructor arguments, and stamps the JNI identity hash on freshly activated peers. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Route generated constructor UCO activation through the invoker type when a proxy represents an abstract or interface peer. Cover both Xamarin.Android and Java.Interop activation constructor styles. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace parameterless-only constructor metadata with general managed-constructor availability, extract repeated constructor UCO emission through helpers, add a tracked PopValue helper, and expand runtime coverage with mixed multi-argument and throwing constructors. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
TypeManager.n_Activatecannot marshal are ignored there, while trimmable typemap validates the new generated UCO pathContribution to the trimmable typemap epic
This PR closes the Java object/constructor activation gap required for trimmable typemap parity. Generated native callbacks can now create or re-enter the correct managed peer for Java-initiated construction instead of falling back to unsupported reflection/native-member registration paths, which moves the trimmable typemap closer to replacing the legacy runtime typemap for app/device execution.
Validation
./dotnet-local.sh test tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests.csproj -v:minimal— 465 passedMSBUILDDISABLENODEREUSE=1 ./dotnet-local.sh msbuild tests/Mono.Android-Tests/Mono.Android-Tests/Mono.Android.NET-Tests.csproj -restore -t:RunTestApp -p:Configuration=Debug -p:_AndroidTypeMapImplementation=llvm-ir -p:UseMonoRuntime=true -p:TestFixture=Java.InteropTests.ConstructorActivationTests -nr:false -v:minimal— constructor fixture passed; legacy-incompatible cases ignored by designMSBUILDDISABLENODEREUSE=1 ./dotnet-local.sh msbuild tests/Mono.Android-Tests/Mono.Android-Tests/Mono.Android.NET-Tests.csproj -restore -t:RunTestApp -p:Configuration=Debug -p:_AndroidTypeMapImplementation=trimmable -p:UseMonoRuntime=false -p:TestFixture=Java.InteropTests.ConstructorActivationTests -nr:false -v:minimal— all 35 constructor activation cases passed; command exits nonzero only because the target still runs unrelated tests andJavaObjectTest.DisposeAccessesThiscurrently fails in the trimmable pathrm -rf src/Mono.Android/obj/Release && ./dotnet-local.sh restore Xamarin.Android.sln -p:Configuration=Release -v:minimal && ./dotnet-local.sh restore src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Xamarin.Android.Build.Tests.csproj -p:Configuration=Release -p:DotNetStableTargetFramework=net10.0 -v:minimal && make all CONFIGURATION=Release MSBUILD_ARGS="--no-restore -m:1"MSBUILDDISABLENODEREUSE=1 ./dotnet-local.sh build tests/Mono.Android-Tests/Mono.Android-Tests/Mono.Android.NET-Tests.csproj -t:RunTestApp -c Release -p:_AndroidTypeMapImplementation=trimmable -p:UseMonoRuntime=false -p:TestFixture=Java.InteropTests.ConstructorActivationTests -nr:false -v:minimal— all 35 constructor activation cases passed; same unrelatedJavaObjectTest.DisposeAccessesThistrimmable failure remains outside this fixtureRelated issues