[TrimmableTypeMap] Compute generated IL maxstack#11260
Conversation
There was a problem hiding this comment.
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
TrackedInstructionEncoderinPEAssemblyBuilderto track IL stack depth, detect underflow, and feed computedmaxstackinto 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
MaxStackvalues.
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. |
simonrozsival
left a comment
There was a problem hiding this comment.
@copilot address code review
Addressed all four review items in commit
All 434 tests still pass. |
|
@copilot try again |
All four code-review changes are ready in local commit The To unblock the push, someone with admin access would need to either:
The pending changes are:
|
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>
d714d55 to
11318c1
Compare
…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
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>
* 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>
Summary
Moves the generated typemap PE emitter from a fixed
DefaultMaxStack = 32to per-method IL evaluation-stack tracking.This PR is based on #11252 /
trimmable-typemap-startup-fixes.InstructionEncoderwith a tracked emitter inPEAssemblyBuildercall,callvirt,newobj,newarr,ldtoken,stobj,ret, etc.) through explicit helpersmax(computed + 4, 8)so generated fat bodies get a computed value with safety padding and the ECMA-335 tiny-method floorValidation
dotnet test tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests.csproj -v minimaldotnet new maui) built as Releasenet11.0-androidforandroid-arm64with CoreCLR/trimmable typemapsemulator-5554com.companyname.mauimaxstacksmoke/crc64d3fac713b535959c.MainActivityStatus: ok,LaunchState: COLD,TotalTime: 1610ms