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

JavaCallableWrapperGenerator uses XML? #540

Open
jonpryor opened this issue Dec 13, 2019 · 2 comments
Open

JavaCallableWrapperGenerator uses XML? #540

jonpryor opened this issue Dec 13, 2019 · 2 comments
Labels
callable-wrappers Issues with Java Callable Wrappers enhancement Proposed change to current functionality

Comments

@jonpryor
Copy link
Member

Context: https://xamarinhq.slack.com/archives/C03CEGRUW/p1574437340441800

Java.Interop.Tools.JavaCallableWrappers.JavaCallableWrapperGenerator reads assemblies via Mono.Cecil in order to write Java Callable Wrappers.

Is this actually desirable?

One of the impacts to this decision is that the Xamarin.Android linker can't fully link; it must preserve [Register] custom attributes so that the Java Callable Wrappers can be generated, and then a later linker step must be run to remove all of the [Register] attributes:

i wonder if the linker could generate a file of "here's the remaining things that need java stubs (with Register data)", then[JavaCallableWrapperGenerator] could work from that file instead of needing to Cecil the assembly, and then linker could remove [Register] at the same time, so we don't need to do that Cecil either
cut out 2 full cecil loads and enumerations

There have also been mutterings of migrating parts of the <GenerateJavaStubs/> task to use System.Reflection.Metadata from Cecil, or to make it suitable for use with Incremental Builds.

It may be desirable to have JavaCallableWrapperGenerator instead process an XML description of the types to generate, possibly using the same XML format as generator's api.xml files, and then update the linker to generate this XML file as part of it's operation.

This would allow JavaCallableWrapperGenerator to not use Mono.Cecil in order to operate, which may be desirable.

@jonpryor jonpryor added the enhancement Proposed change to current functionality label Dec 13, 2019
@jpobst
Copy link
Contributor

jpobst commented Dec 14, 2019

Just some notes that were raised:

  • The linker (generally) only runs on Release builds, so this would not help Debug builds
  • We would still need to use Cecil (or something) for Debug builds to build the wrappers

@jonpryor
Copy link
Member Author

More recent random thoughts…

We're now ".NET", not Xamarin.Android, and as part of that migration we now use the .NET linker in the Release configuration.

The .NET linker runs a pipeline of linker steps, which .NET Android controls via the @(_TrimmerCustomSteps) item group.

A thought of ours, to "force" cleanup of the <GenerateJavaStubs/> task, is to move Java Callable Wrapper generation into a linker pipeline step. (This feels quite plausible and sane.)

.NET Android Debug builds also need to do "linker-like" things, such as GC.KeepAlive() insertion for older binding assemblies and fixing "missing" abstract methods, so it occurs to us that Debug builds could also use a (shorter!) linker pipeline, which could also be used to emit Java Callable Wrappers. (The big unknown here is performance: how much slower would this be? Could we have an "in-process linker pipeline"?)

If these ideas pan out, it means that this idea is moot: there wouldn't need to be an XML intermediary, because Java Callable Wrapper generation would be a linker step which could run before [Register] is removed, and then [Register] could be removed in a subsequent linker step in the same pipeline. Furthermore, the .NET linker already uses Cecil, so use of Cecil would not be a new concern.

jonpryor pushed a commit that referenced this issue Feb 21, 2024
Context: #540

Refactor `Java.Interop.Tools.JavaCallableWrappers` to be more
maintainable and modular.  Adopts the same architecture as
`generator`: using a `CecilImporter` to import data from a compiled
assembly into an intermediate model.  Java code is then generated
from the intermediate model.

Refactoring and updating this code should make it not only more
maintainable, but also easier to extend with potential upcoming
features we are considering:

  - Moving `jcw-gen` into the `ILLink` pipeline
  - [Sanity-check Java Callable Wrappers][0]
  - Possibly creating a Kotlin backend

[0]: dotnet/android#4278
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
callable-wrappers Issues with Java Callable Wrappers enhancement Proposed change to current functionality
Projects
None yet
Development

No branches or pull requests

2 participants