Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[linker] Add AddKeepAlivesStep #5278

Merged
merged 11 commits into from
Dec 18, 2020

Conversation

brendanzagaeski
Copy link
Contributor

@brendanzagaeski brendanzagaeski commented Nov 9, 2020

Context: dotnet/java-interop#719

Modify "old" bindings, so that they hold the object reference
long enough to avoid above issue.

It is implemented as linker step, which looks for offended methods
and injects call to System.GC.KeepAlive to prevent premature
collection.

The injection can be disabled with AndroidAddKeepAlives property
set to false.

The property's default value depends on AndroidIncludeDebugSymbols
property and is thus true for release builds and false for debug.

The impact on release build of XA forms template (flyout variety) is
cca 137ms on my machine.

Disabled:

12079 ms  LinkAssemblies                             1 calls

Enabled:

12216 ms  LinkAssemblies                             1 calls

Example updated method body:

try {
	JniArgumentValue* ptr = stackalloc JniArgumentValue[1];
	*ptr = new JniArgumentValue((windowToken == null) ? IntPtr.Zero : ((Java.Lang.Object)windowToken).Handle);
	ptr[1] = new JniArgumentValue((int)flags);
	bool result = JniPeerMembers.InstanceMethods.InvokeAbstractBooleanMethod("...", this, ptr);
} finally {
	GC.KeepAlive(windowToken);
	return result;
}

Added new AndroidAddKeepAlives test to LinkerTests.cs to check
whether registered method was updated.

@radekdoulik radekdoulik marked this pull request as ready for review December 16, 2020 15:27
@jonpryor jonpryor merged commit e88cfbc into dotnet:master Dec 18, 2020
@github-actions github-actions bot locked and limited conversation to collaborators Jan 24, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants