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.

Sign up for GitHub

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

Jump to bottom

Java Callable Wrapper class visibility should match C# class visibility. #754

Open
jonpryor opened this issue Dec 4, 2020 · 0 comments
Open

Java Callable Wrapper class visibility should match C# class visibility. #754

jonpryor opened this issue Dec 4, 2020 · 0 comments
Labels
callable-wrappers Issues with Java Callable Wrappers enhancement Proposed change to current functionality

Comments

@jonpryor
Copy link
Member

jonpryor commented Dec 4, 2020

I don't see why the Java Callable Wrapper for an internal C# class would result in a public Java Callable Wrapper class.
@jpobst jpobst added callable-wrappers Issues with Java Callable Wrappers enhancement Proposed change to current functionality labels Dec 9, 2020
jonpryor pushed a commit to dotnet/android that referenced this issue Dec 11, 2020
@radekdoulik
[One .NET] Don't preserve all of Android.Runtime.JavaObject (#5367)
d0df243 
Context: #5167

The `Android.Runtime.JavaObject` type is not accessed by reflection,
and does not need to be fully preserved by the linker.

After allowing `JavaObject` to be linked, we discovered that the
presence of the `JavaObject.Clone()` method pulled in a lot of API,
through the `System.ICloneable` interface.

Removing the `JavaObject.Clone()` method allows us to save more than
900KB in compressed assembly size:

	Summary:
	  -     942,460 Assemblies -43.72% (of 2,155,792)
	  -     947,763 Package size difference -10.50% (of 9,024,291)

Which raises an important question: do we *need* `JavaObject.Clone()`?

We believe the answer is *no*: `JavaObject` is a sealed type; no
*managed* code can call `JavaObject.Clone()`.

This leaves the Java Callable Wrapper, which [*oddly*][0] is `public`,
*and* overrides `java.lang.Object.clone()` as a public method.

Thus, *in theory*, `JavaObject.Clone()` *is* invocable from Java code.

However, *in practice*, to do so the Java code would need to take
a compile-time dependency on `mono.android.jar`, which feels *highly*
unlikely and unusual.  (`java.lang.Object.clone()` is `protected` by
default, and is thus not invocable normally.)

We thus deem it safe to entirely remove `JavaObject.Clone()`.

In the end we are now smaller in assembly size comparing simple XA app
"legacy" and net6:

	Summary:
	  -     356,408 Assemblies -27.69% (of 1,287,165)
	  +   2,646,694 Package size difference 51.43% (of 5,146,413)

And still a bit larger comparing XA XForms app:

	Summary:
	  +   1,001,148 Assemblies 21.01% (of 4,765,097)
	  +   4,013,739 Package size difference 37.75% (of 10,633,264)

[0]: dotnet/java-interop#754
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
callable-wrappers Issues with Java Callable Wrappers enhancement Proposed change to current functionality
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@jonpryor @jpobst