Skip to content

[TrimmableTypeMap] Compute generated IL maxstack#11260

Merged
simonrozsival merged 4 commits into
mainfrom
trimmable-typemap-computed-maxstack
May 12, 2026
Merged

[TrimmableTypeMap] Compute generated IL maxstack#11260
simonrozsival merged 4 commits into
mainfrom
trimmable-typemap-computed-maxstack

Conversation

@simonrozsival
Copy link
Copy Markdown
Member

@simonrozsival simonrozsival commented May 1, 2026

Summary

Moves the generated typemap PE emitter from a fixed DefaultMaxStack = 32 to per-method IL evaluation-stack tracking.

This PR is based on #11252 / trimmable-typemap-startup-fixes.

  • wraps InstructionEncoder with a tracked emitter in PEAssemblyBuilder
  • routes variable-stack IL (call, callvirt, newobj, newarr, ldtoken, stobj, ret, etc.) through explicit helpers
  • seeds catch handlers with stack depth 1 for the exception object
  • emits max(computed + 4, 8) so generated fat bodies get a computed value with safety padding and the ECMA-335 tiny-method floor
  • throws on stack underflow while generating IL so missing stack modeling fails early

Validation

  • dotnet test tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests.csproj -v minimal
    • 434 total, 0 failed
  • Fresh MAUI template app (dotnet new maui) built as Release net11.0-android for android-arm64 with CoreCLR/trimmable typemaps
    • installed on emulator-5554
    • cold-started com.companyname.mauimaxstacksmoke/crc64d3fac713b535959c.MainActivity
    • launch completed with Status: ok, LaunchState: COLD, TotalTime: 1610ms
    • process stayed alive and foreground; logcat showed no fatal exception or native crash

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

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 PE/IL emitter to compute per-method maxstack by tracking evaluation stack usage during IL generation, replacing the previous fixed maxstack approach.

Changes:

  • Introduces a TrackedInstructionEncoder in PEAssemblyBuilder to track IL stack depth, detect underflow, and feed computed maxstack into method body emission.
  • Updates IL emission sites to route stack-affecting instructions through tracked helper methods (e.g., Call, Callvirt, NewObject, NewArray, LoadToken, Return), including explicit catch-handler stack seeding.
  • Adds tests that assert generated methods now use computed MaxStack values.

Reviewed changes

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

Show a summary per file
File Description
tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/Generator/TypeMapAssemblyGeneratorTests.cs Adds assertions for emitted MaxStack and updates one IL emission call to use the new tracked Return() helper.
tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/Generator/RootTypeMapAssemblyGeneratorTests.cs Adds a helper to find methods by name and tests that Initialize uses computed MaxStack.
src/Microsoft.Android.Sdk.TrimmableTypeMap/Generator/TypeMapAssemblyEmitter.cs Switches method-body emission callbacks to use TrackedInstructionEncoder and updates many IL sequences to tracked helpers.
src/Microsoft.Android.Sdk.TrimmableTypeMap/Generator/RootTypeMapAssemblyGenerator.cs Updates emitted IL for Initialize to use tracked Call/NewArray/Return helpers.
src/Microsoft.Android.Sdk.TrimmableTypeMap/Generator/PEAssemblyBuilder.cs Replaces fixed maxstack with tracked stack accounting and passes computed maxstack into MethodBodyStreamEncoder.AddMethodBody.

Copy link
Copy Markdown
Member Author

@simonrozsival simonrozsival left a comment

Choose a reason for hiding this comment

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

@copilot address code review

Copy link
Copy Markdown
Contributor

Copilot AI commented May 3, 2026

@copilot address code review

Addressed all four review items in commit 0ecd840:

  1. Branch() fix — removed Br/Br_s from the SetStack(0) case (those opcodes don't clear the evaluation stack, unlike leave). They now fall through to the default throw, same as any other unsupported opcode. Since Br/Br_s are not used anywhere in the current emitter, this also serves as a correctness guard if they're ever added.

  2. Test assertions — switched all three hard-coded Assert.Equal(8, ...) checks to looser variants:

    • Generate_AcwProxy_RegisterNativesUsesComputedMaxStack: Assert.InRange(body.MaxStack, 5, 16)
    • Generate_InitializeUsesComputedMaxStack (root tests): Assert.InRange(body.MaxStack, 5, 16) (also dropped the now-unnecessary expectedMaxStack [InlineData] parameter)
    • Generate_UcoConstructor_UsesComputedMaxStack: Assert.True(body.MaxStack > 1, ...)

All 434 tests still pass.

@simonrozsival
Copy link
Copy Markdown
Member Author

@copilot try again

Copy link
Copy Markdown
Contributor

Copilot AI commented May 3, 2026

@copilot try again

All four code-review changes are ready in local commit d1e3585 and all 434 unit tests pass, but the push is blocked by a GitHub repository ruleset ("Require checks for auto-merge") that requires Xamarin.Android-PR to pass before any push to this branch is accepted.

The Xamarin.Android-PR check has been in failure state for commit ef30f6f since May 1 (build 13991688), and the base branch trimmable-typemap-startup-fixes has also been failing the same check since around the same time (build 13991851, 13996296, 13999576). This appears to be a pre-existing CI issue unrelated to the maxstack changes.

To unblock the push, someone with admin access would need to either:

  1. Push the commit directly from a local clone (admin access bypasses the ruleset), or
  2. Retrigger / fix the Xamarin.Android-PR CI on the base branch so it passes.

The pending changes are:

  • src/Microsoft.Android.Sdk.TrimmableTypeMap/Generator/PEAssemblyBuilder.cs — remove Br/Br_s from the SetStack(0) case
  • tests/.../RootTypeMapAssemblyGeneratorTests.csAssert.InRange(body.MaxStack, 5, 16), drop expectedMaxStack parameter
  • tests/.../TypeMapAssemblyGeneratorTests.csAssert.InRange(5, 16) and Assert.True(body.MaxStack > 1, ...)

simonrozsival and others added 2 commits May 4, 2026 20:33
Track emitted IL stack depth in PEAssemblyBuilder instead of using a fixed maxstack of 32, and emit max(computed + 4, 8).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Use less brittle maxstack test assertions and reject unsupported unconditional branches in the tracked IL encoder.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@simonrozsival simonrozsival force-pushed the trimmable-typemap-computed-maxstack branch from d714d55 to 11318c1 Compare May 4, 2026 18:33
…puted-maxstack

# Conflicts:
#	src/Microsoft.Android.Sdk.TrimmableTypeMap/Generator/RootTypeMapAssemblyGenerator.cs
#	src/Microsoft.Android.Sdk.TrimmableTypeMap/Generator/TypeMapAssemblyEmitter.cs
# Conflicts:
#	src/Microsoft.Android.Sdk.TrimmableTypeMap/Generator/RootTypeMapAssemblyGenerator.cs
@simonrozsival simonrozsival merged commit 7610b56 into main May 12, 2026
3 checks passed
@simonrozsival simonrozsival deleted the trimmable-typemap-computed-maxstack branch May 12, 2026 05:05
@simonrozsival simonrozsival added copilot `copilot-cli` or other AIs were used to author this trimmable-type-map labels May 12, 2026
simonrozsival added a commit that referenced this pull request May 12, 2026
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>
simonrozsival added a commit that referenced this pull request May 13, 2026
* Fix trimmable Java object activation

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>

* Generate direct default constructor activation

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>

* Port default-ctor nctor IL to TrackedInstructionEncoder

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>

* Implement trimmable constructor activation parity

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>

* Honor invoker activation in constructor UCOs

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>

* Address constructor activation review feedback

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>

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants