Skip to content

Avoid keepalive-only finally blocks in generated bindings - #12343

Open
jonathanpeppers wants to merge 5 commits into
dotnet:mainfrom
jonathanpeppers:jonathanpeppers-optimize-binding-keepalive
Open

Avoid keepalive-only finally blocks in generated bindings#12343
jonathanpeppers wants to merge 5 commits into
dotnet:mainfrom
jonathanpeppers:jonathanpeppers-optimize-binding-keepalive

Conversation

@jonathanpeppers

@jonathanpeppers jonathanpeppers commented Aug 11, 2026

Copy link
Copy Markdown
Member

Generated binding methods currently use exception-handling regions even when their finally block only calls GC.KeepAlive. These regions add metadata and prevent RyuJIT from inlining otherwise-thin JNI wrappers.

This changes methods, constructors, and field-backed property setters to emit straight-line JNI invocation, GC.KeepAlive, and return sequences when no exceptional-path cleanup exists. Bindings that dispose marshaler state or delete local JNI references retain try/finally, and compatibility virtual methods retain their NoSuchMethodError translation. Generated baselines and targeted coverage verify both keep-alive-only and real-cleanup paths across JavaInterop1 and XAJavaInterop1.

Fixes #12314

Pull Request
title and
description
should follow the
commit-messages.md workflow documentation, and in particular should include:

  • Useful description of why the change is necessary.
  • Links to issues fixed
  • Unit tests

Testing

dotnet test external/Java.Interop/tests/generator-Tests/generator-Tests.csproj (466 passed)

jonathanpeppers and others added 2 commits August 7, 2026 14:54
@jonathanpeppers
Avoid keepalive-only finally blocks in bindings
2e52794 
Emit straight-line GC.KeepAlive calls when generated JNI bindings have no exceptional-path cleanup, while preserving try/finally for real marshaling cleanup.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@jonathanpeppers
Test compat virtual parameter cleanup
d0bbea4 
Cover keepalive-only object parameters and string marshaling cleanup in compatibility virtual methods for both generator targets.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: c892727a-ae15-4b3f-9c29-d6b9c3b89401
Copilot AI lite review requested due to automatic review settings August 11, 2026 12:08
Copilot AI reviewed Aug 11, 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 Java.Interop binding generator to avoid emitting try/finally regions when the finally block would only contain GC.KeepAlive(...), reducing EH metadata and enabling better JIT inlining of thin JNI wrappers. It also refreshes generator baselines and adds/extends tests to validate both the EH-free keepalive path and the retained-cleanup/compat paths.

Changes:

  • Emit straight-line invocation + GC.KeepAlive (and deferred return when needed) when there is no exceptional-path cleanup.
  • Retain try/finally when marshaling cleanup is required, and retain compat-virtual NoSuchMethodError translation.
  • Update generator expected-output baselines and add targeted assertions in unit/integration tests.

Reviewed changes

Copilot reviewed 128 out of 128 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
external/Java.Interop/tools/generator/SourceWriters/Extensions/SourceWriterExtensions.cs Core generator logic to elide keepalive-only finally blocks and optionally defer returns.
external/Java.Interop/tools/generator/SourceWriters/BoundFieldAsProperty.cs Update field-backed property setter emission to avoid keepalive-only finally.
external/Java.Interop/tools/generator/SourceWriters/BoundConstructor.cs Update constructor emission to avoid keepalive-only finally.
external/Java.Interop/tests/generator-Tests/Unit-Tests/DefaultInterfaceMethodsTests.cs Add extra structural assertion over generated output.
external/Java.Interop/tests/generator-Tests/Unit-Tests/CodeGeneratorTests.cs Extend compat-virtual coverage and assert new keepalive/cleanup shapes.
external/Java.Interop/tests/generator-Tests/Unit-Tests/CodeGeneratorExpectedResults/XAJavaInterop1/WriteStaticInterfaceProperty.txt Baseline update for EH-free keepalive-only output.
external/Java.Interop/tests/generator-Tests/Unit-Tests/CodeGeneratorExpectedResults/XAJavaInterop1/WriteStaticInterfaceMethod.txt Baseline update for EH-free keepalive-only output.
external/Java.Interop/tests/generator-Tests/Unit-Tests/CodeGeneratorExpectedResults/XAJavaInterop1/WriteKotlinUnsignedTypePropertiesClass.txt Baseline update for EH-free keepalive-only output.
external/Java.Interop/tests/generator-Tests/Unit-Tests/CodeGeneratorExpectedResults/XAJavaInterop1/WriteKotlinUnsignedTypeMethodsClass.txt Baseline update for EH-free keepalive-only output.
external/Java.Interop/tests/generator-Tests/Unit-Tests/CodeGeneratorExpectedResults/XAJavaInterop1/WriteKotlinUnsignedArrayTypePropertiesClass.txt Baseline update for EH-free keepalive-only output.
external/Java.Interop/tests/generator-Tests/Unit-Tests/CodeGeneratorExpectedResults/XAJavaInterop1/WriteInterfaceFieldAsDimProperty.txt Baseline update for EH-free keepalive-only output.
external/Java.Interop/tests/generator-Tests/Unit-Tests/CodeGeneratorExpectedResults/XAJavaInterop1/WriteInterfaceDefaultPropertyGetterOnly.txt Baseline update for EH-free keepalive-only output.
external/Java.Interop/tests/generator-Tests/Unit-Tests/CodeGeneratorExpectedResults/XAJavaInterop1/WriteInterfaceDefaultProperty.txt Baseline update for EH-free keepalive-only output.
external/Java.Interop/tests/generator-Tests/Unit-Tests/CodeGeneratorExpectedResults/XAJavaInterop1/WriteInterfaceDefaultMethod.txt Baseline update for EH-free keepalive-only output.
external/Java.Interop/tests/generator-Tests/Unit-Tests/CodeGeneratorExpectedResults/XAJavaInterop1/WriteInterface.txt Baseline update for EH-free keepalive-only output.
external/Java.Interop/tests/generator-Tests/Unit-Tests/CodeGeneratorExpectedResults/XAJavaInterop1/WriteDefaultInterfaceMethodInvoker.txt Baseline update for EH-free keepalive-only output.
external/Java.Interop/tests/generator-Tests/Unit-Tests/CodeGeneratorExpectedResults/XAJavaInterop1/WriteConstSugarInterfaceFields.txt Baseline update for EH-free keepalive-only output.
external/Java.Interop/tests/generator-Tests/Unit-Tests/CodeGeneratorExpectedResults/XAJavaInterop1/WriteClass.txt Baseline update for EH-free keepalive-only output.
external/Java.Interop/tests/generator-Tests/Unit-Tests/CodeGeneratorExpectedResults/XAJavaInterop1/ObsoleteInterfaceAlternativeClass.txt Baseline update for EH-free keepalive-only output.
external/Java.Interop/tests/generator-Tests/Unit-Tests/CodeGeneratorExpectedResults/XAJavaInterop1-NRT/WriteInterface.txt Baseline update for EH-free keepalive-only output (NRT).
external/Java.Interop/tests/generator-Tests/Unit-Tests/CodeGeneratorExpectedResults/XAJavaInterop1-NRT/WriteClass.txt Baseline update for EH-free keepalive-only output (NRT).
external/Java.Interop/tests/generator-Tests/Unit-Tests/CodeGeneratorExpectedResults/JavaInterop1/WriteObjectField.txt Baseline update: move keepalive out of keepalive-only finally.
external/Java.Interop/tests/generator-Tests/Unit-Tests/CodeGeneratorExpectedResults/JavaInterop1/WriteKotlinUnsignedTypePropertiesClass.txt Baseline update for EH-free keepalive-only output.
external/Java.Interop/tests/generator-Tests/Unit-Tests/CodeGeneratorExpectedResults/JavaInterop1/WriteKotlinUnsignedTypeMethodsClass.txt Baseline update for EH-free keepalive-only output.
external/Java.Interop/tests/generator-Tests/Unit-Tests/CodeGeneratorExpectedResults/JavaInterop1/WriteKotlinUnsignedArrayTypePropertiesClass.txt Baseline update for EH-free keepalive-only output.
external/Java.Interop/tests/generator-Tests/Unit-Tests/CodeGeneratorExpectedResults/JavaInterop1/WriteInterface.txt Baseline update for EH-free keepalive-only output.
external/Java.Interop/tests/generator-Tests/Unit-Tests/CodeGeneratorExpectedResults/JavaInterop1/WriteDuplicateInterfaceEventArgs.txt Baseline update for EH-free keepalive-only output.
external/Java.Interop/tests/generator-Tests/Unit-Tests/CodeGeneratorExpectedResults/JavaInterop1/WriteClass.txt Baseline update for EH-free keepalive-only output.
external/Java.Interop/tests/generator-Tests/Integration-Tests/NormalProperties.cs Add integration assertion for keepalive-only setter shape.
external/Java.Interop/tests/generator-Tests/Integration-Tests/NormalMethods.cs Add integration assertions for keepalive-only and cleanup-retained shapes.
external/Java.Interop/tests/generator-Tests/expected.xaji/TestInterface/Test.ME.TestInterfaceImplementation.cs Updated expected generated output (XAJavaInterop1) for keepalive-only paths.
external/Java.Interop/tests/generator-Tests/expected.xaji/TestInterface/Test.ME.ITestInterface.cs Updated expected generated output (XAJavaInterop1) for keepalive-only paths.
external/Java.Interop/tests/generator-Tests/expected.xaji/TestInterface/Test.ME.IGenericPropertyInterface.cs Updated expected generated output (XAJavaInterop1) for keepalive-only paths.
external/Java.Interop/tests/generator-Tests/expected.xaji/TestInterface/Test.ME.GenericStringPropertyImplementation.cs Updated expected generated output (XAJavaInterop1) for keepalive-only paths.
external/Java.Interop/tests/generator-Tests/expected.xaji/TestInterface/Test.ME.GenericStringImplementation.cs Updated expected generated output (XAJavaInterop1) for keepalive-only paths.
external/Java.Interop/tests/generator-Tests/expected.xaji/TestInterface/Test.ME.GenericObjectPropertyImplementation.cs Updated expected generated output (XAJavaInterop1) for keepalive-only paths.
external/Java.Interop/tests/generator-Tests/expected.xaji/TestInterface/Test.ME.GenericImplementation.cs Updated expected generated output (XAJavaInterop1) for keepalive-only paths.
external/Java.Interop/tests/generator-Tests/expected.xaji/TestInterface/Java.Util.IQueue.cs Updated expected generated output (XAJavaInterop1) for keepalive-only paths.
external/Java.Interop/tests/generator-Tests/expected.xaji/TestInterface/Java.Util.IDeque.cs Updated expected generated output (XAJavaInterop1) for keepalive-only paths.
external/Java.Interop/tests/generator-Tests/expected.xaji/TestInterface/Java.Util.ICollection.cs Updated expected generated output (XAJavaInterop1) for keepalive-only paths.
external/Java.Interop/tests/generator-Tests/expected.xaji/TestInterface/IInterfaceWithoutNamespace.cs Updated expected generated output (XAJavaInterop1) for keepalive-only paths.
external/Java.Interop/tests/generator-Tests/expected.xaji/TestInterface/ClassWithoutNamespace.cs Updated expected generated output (XAJavaInterop1) for keepalive-only paths.
external/Java.Interop/tests/generator-Tests/expected.xaji/Streams/Java.Lang.Throwable.cs Updated expected generated output (XAJavaInterop1) for keepalive-only paths.
external/Java.Interop/tests/generator-Tests/expected.xaji/Streams/Java.IO.OutputStream.cs Updated expected generated output (XAJavaInterop1) for keepalive-only paths.
external/Java.Interop/tests/generator-Tests/expected.xaji/Streams/Java.IO.IOException.cs Updated expected generated output (XAJavaInterop1) for keepalive-only paths.
external/Java.Interop/tests/generator-Tests/expected.xaji/Streams/Java.IO.InputStream.cs Updated expected generated output (XAJavaInterop1) for keepalive-only paths.
external/Java.Interop/tests/generator-Tests/expected.xaji/Streams/Java.IO.FilterOutputStream.cs Updated expected generated output (XAJavaInterop1) for keepalive-only paths.
external/Java.Interop/tests/generator-Tests/expected.ji/TestInterface/Test.ME.TestInterfaceImplementation.cs Updated expected generated output (JavaInterop1) for keepalive-only paths.
external/Java.Interop/tests/generator-Tests/expected.ji/TestInterface/Test.ME.ITestInterface.cs Updated expected generated output (JavaInterop1) for keepalive-only paths.
external/Java.Interop/tests/generator-Tests/expected.ji/TestInterface/Test.ME.IGenericPropertyInterface.cs Updated expected generated output (JavaInterop1) for keepalive-only paths.
external/Java.Interop/tests/generator-Tests/expected.ji/TestInterface/Test.ME.IGenericInterface.cs Updated expected generated output (JavaInterop1) for keepalive-only paths.
external/Java.Interop/tests/generator-Tests/expected.ji/TestInterface/Test.ME.GenericStringPropertyImplementation.cs Updated expected generated output (JavaInterop1) for keepalive-only paths.
external/Java.Interop/tests/generator-Tests/expected.ji/TestInterface/Test.ME.GenericStringImplementation.cs Updated expected generated output (JavaInterop1) for keepalive-only paths.
external/Java.Interop/tests/generator-Tests/expected.ji/TestInterface/Test.ME.GenericObjectPropertyImplementation.cs Updated expected generated output (JavaInterop1) for keepalive-only paths.
external/Java.Interop/tests/generator-Tests/expected.ji/TestInterface/Test.ME.GenericImplementation.cs Updated expected generated output (JavaInterop1) for keepalive-only paths.
external/Java.Interop/tests/generator-Tests/expected.ji/TestInterface/Java.Util.IQueue.cs Updated expected generated output (JavaInterop1) for keepalive-only paths.
external/Java.Interop/tests/generator-Tests/expected.ji/TestInterface/Java.Util.IDeque.cs Updated expected generated output (JavaInterop1) for keepalive-only paths.
external/Java.Interop/tests/generator-Tests/expected.ji/TestInterface/Java.Util.ICollection.cs Updated expected generated output (JavaInterop1) for keepalive-only paths.
external/Java.Interop/tests/generator-Tests/expected.ji/TestInterface/IInterfaceWithoutNamespace.cs Updated expected generated output (JavaInterop1) for keepalive-only paths.
external/Java.Interop/tests/generator-Tests/expected.ji/TestInterface/ClassWithoutNamespace.cs Updated expected generated output (JavaInterop1) for keepalive-only paths.
external/Java.Interop/tests/generator-Tests/expected.ji/Streams/Java.Lang.Throwable.cs Updated expected generated output (JavaInterop1) for keepalive-only paths.
external/Java.Interop/tests/generator-Tests/expected.ji/Streams/Java.IO.OutputStream.cs Updated expected generated output (JavaInterop1) for keepalive-only paths.
external/Java.Interop/tests/generator-Tests/expected.ji/Streams/Java.IO.IOException.cs Updated expected generated output (JavaInterop1) for keepalive-only paths.
external/Java.Interop/tests/generator-Tests/expected.ji/Streams/Java.IO.InputStream.cs Updated expected generated output (JavaInterop1) for keepalive-only paths.
external/Java.Interop/tests/generator-Tests/expected.ji/Streams/Java.IO.FilterOutputStream.cs Updated expected generated output (JavaInterop1) for keepalive-only paths.
external/Java.Interop/tests/generator-Tests/expected.ji/java.util.List/Xamarin.Test.SomeObject.cs Updated expected generated output (JavaInterop1) for keepalive-only paths.
external/Java.Interop/tests/generator-Tests/expected.ji/java.lang.Enum/Java.Lang.IComparable.cs Updated expected generated output (JavaInterop1) for keepalive-only paths.
external/Java.Interop/tests/generator-Tests/expected.ji/java.lang.Enum/Java.Lang.Enum.cs Updated expected generated output (JavaInterop1) for keepalive-only paths.
external/Java.Interop/tests/generator-Tests/expected.ji/ParameterXPath/Xamarin.Test.A.cs Updated expected generated output (JavaInterop1) for keepalive-only paths.
external/Java.Interop/tests/generator-Tests/expected.ji/Arrays/Xamarin.Test.SomeObject.cs Updated expected generated output (JavaInterop1) for keepalive-only paths.
external/Java.Interop/tests/generator-Tests/expected.ji/InterfaceMethodsConflict/Xamarin.Test.SomeObject2.cs Updated expected generated output (JavaInterop1) for keepalive-only paths.
external/Java.Interop/tests/generator-Tests/expected.ji/InterfaceMethodsConflict/Xamarin.Test.SomeObject.cs Updated expected generated output (JavaInterop1) for keepalive-only paths.
external/Java.Interop/tests/generator-Tests/expected.ji/InterfaceMethodsConflict/Xamarin.Test.II2.cs Updated expected generated output (JavaInterop1) for keepalive-only paths.
external/Java.Interop/tests/generator-Tests/expected.ji/InterfaceMethodsConflict/Xamarin.Test.II1.cs Updated expected generated output (JavaInterop1) for keepalive-only paths.
external/Java.Interop/tests/generator-Tests/expected.ji/CSharpKeywords/Xamarin.Test.CSharpKeywords.cs Updated expected generated output (JavaInterop1) for keepalive-only paths.
external/Java.Interop/tests/generator-Tests/expected.ji/CSharpKeywords/Java.Lang.Throwable.cs Updated expected generated output (JavaInterop1) for keepalive-only paths.
external/Java.Interop/tests/generator-Tests/expected.ji/Constructors/Xamarin.Test.SomeObject2.cs Updated expected generated output (JavaInterop1) for keepalive-only paths.
external/Java.Interop/tests/generator-Tests/expected.ji/Constructors/Xamarin.Test.SomeObject.cs Updated expected generated output (JavaInterop1) for keepalive-only paths.
external/Java.Interop/tests/generator-Tests/expected.ji/NestedTypes/Xamarin.Test.NotificationCompatBase.cs Updated expected generated output (JavaInterop1) for keepalive-only paths.
external/Java.Interop/tests/generator-Tests/expected.ji/Adapters/Xamarin.Test.GenericReturnObject.cs Updated expected generated output (JavaInterop1) for keepalive-only paths.
external/Java.Interop/tests/generator-Tests/expected.ji/Adapters/Xamarin.Test.AdapterView.cs Updated expected generated output (JavaInterop1) for keepalive-only paths.
external/Java.Interop/tests/generator-Tests/expected.ji/Adapters/Xamarin.Test.AbsSpinner.cs Updated expected generated output (JavaInterop1) for keepalive-only paths.
external/Java.Interop/tests/generator-Tests/expected.ji/AccessModifiers/Xamarin.Test.TestClass.cs Updated expected generated output (JavaInterop1) for keepalive-only paths.
external/Java.Interop/tests/generator-Tests/expected.ji/AccessModifiers/Xamarin.Test.PublicFinalClass.cs Updated expected generated output (JavaInterop1) for keepalive-only paths.
external/Java.Interop/tests/generator-Tests/expected.ji/AccessModifiers/Xamarin.Test.PublicClass.cs Updated expected generated output (JavaInterop1) for keepalive-only paths.
external/Java.Interop/tests/generator-Tests/expected.ji/AccessModifiers/Xamarin.Test.IExtendedInterface.cs Updated expected generated output (JavaInterop1) for keepalive-only paths.
external/Java.Interop/tests/generator-Tests/expected.ji/AccessModifiers/Xamarin.Test.ExtendPublicClass.cs Updated expected generated output (JavaInterop1) for keepalive-only paths.
external/Java.Interop/tests/generator-Tests/expected.ji/AccessModifiers/Xamarin.Test.BasePublicClass.cs Updated expected generated output (JavaInterop1) for keepalive-only paths.

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

Comment thread external/Java.Interop/tools/generator/SourceWriters/Extensions/SourceWriterExtensions.cs
@jonathanpeppers
Merge origin/main into keepalive optimization
5c0901b 
Resolve the generator source-writer overlap after legacy interface invoker support was removed, and refresh the affected expected outputs.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: c892727a-ae15-4b3f-9c29-d6b9c3b89401
@jonathanpeppers
Preserve compat exception argument lifetimes
c03b30e 
Emit keep-alive calls before translating NoSuchMethodError when no cleanup finally is required, preserving the previous caught-exception lifetime behavior.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: c892727a-ae15-4b3f-9c29-d6b9c3b89401
@jonathanpeppers

jonathanpeppers commented Aug 13, 2026

Copy link
Copy Markdown
Member Author

/review

@github-actions

github-actions Bot commented Aug 13, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

✅ Android PR Reviewer completed successfully!

Generated by Android PR Reviewer for #12343

github-actions[bot]
github-actions Bot reviewed Aug 13, 2026
View reviewed changes

@github-actions github-actions Bot 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.

✅ LGTM from code review — 0 errors, 0 warnings, 1 suggestion.

The cleanup/keepalive split is applied consistently across methods, constructors, and field-backed setters. Real marshaling cleanup remains protected by finally, non-void keepalive-only methods defer the return correctly, and compatibility virtual methods preserve argument lifetime on the translated NoSuchMethodError path. The regression coverage exercises straight-line keepalives, retained cleanup blocks, and balanced compatibility control flow.

The focused Java.Interop checks are passing on macOS and Windows; the broader Linux, macOS, and Windows platform builds are still in progress.

Generated by Android PR Reviewer for #12343 · gpt56 · 148.3 AIC · ⌖ 8.81 AIC · ⊞ 25.6K
Comment /review to run again

Comment thread external/Java.Interop/tools/generator/SourceWriters/Extensions/SourceWriterExtensions.cs Outdated
@jonathanpeppers
Avoid intermediate keep-alive collections
ba294be 
Scan parameter lists directly when deciding and emitting generated cleanup and keep-alive statements, avoiding per-member temporary collections while preserving generated output.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: c892727a-ae15-4b3f-9c29-d6b9c3b89401
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@github-actions github-actions[bot] github-actions[bot] left review comments
Copilot code review Copilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@simonrozsival simonrozsival Awaiting requested review from simonrozsival simonrozsival is a code owner

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Avoid try/finally for keep-alive-only generated bindings

2 participants

@jonathanpeppers