native icon indicating copy to clipboard operation
native copied to clipboard

Possibly convert all `JObject`s to extension types?

Open HosseinYousefi opened this issue 1 year ago • 13 comments

Opening an issue to track this experiment. I'm not sure if this is possible to do but it might reduce the complexity of the generated code, and possibly remove "type classes" from the code. The reason we have type classes is to construct arbitrary JObjects, but we can construct one and just cast them if they're all extension types!

HosseinYousefi avatar Oct 07 '24 15:10 HosseinYousefi

From a discussion today: This will not play well with making certain JObjects implement certain Dart interfaces. We could not make a JHashMap implement Map.

With user-transformations we want to bring what we do with maps and list in package:jni currently to many more places.

dcharkes avatar Jul 17 '25 12:07 dcharkes

Reopening this to reconsider what our design goals are for JNIgen.

Background

When we initially designed JNIgen, we thought it's a good idea to make it seamless to use Java classes as we would use Dart classes. We of course can't do this all the time but we can get closer to this ideal. One such design decision is representing Java generics as Dart generics.

Another usability improvement came from having JList<T> mix in ListMixin<T>, this makes working with Java lists very intuitive for Dart users and allowed users to do things like iterate over the items of a list in a for loop.

A blocker here used to be: given a generic type T, how could we construct an object of this exact type? For example the operator[] of JList<T> should return a T but we can't do T.fromReference(/* the actual JNI call */).

To solve this problem we created another tree of objects that we call "type classes". JNIgen generates a class named $Foo$Type$ for every class Foo that is accessible through the static getter Foo.type. This class has an instance method fromReference that creates a Foo object from a reference. Now creating a JList<Foo> means that we have to pass Foo.type in the constructor. Then when it comes to actually returning T from say operator [] we can use the passed type class to construct the object we want to return: elementType.fromReference(/* the actual JNI call */).

(Of course the types can be nested for example to create a 2D list of Foo you use the type JList.type(JFoo.type)))

Problem

This is all good but it makes the code generator overly complicated. Every feature added has to play nicely with the type classes. For example adding nullability meant that we now generate $Foo$NullableType$ accessible via the static getter Foo.nullableType. There are other things to keep in mind if we want to generate the Java super interfaces of each class in Dart as well.

Then there is the problem that we're basically adding this new parameter to fix a Dart problem. Because we don't have static polymorphism. We don't want the user to pay this cost for a simple generic method calls. So we created a way to infer the type class from the runtime type class of the passed method parameters. This inference has to play nicely with the added nullability in the Dart type system which can cause bugs like https://github.com/dart-lang/native/issues/2543, and usability issues like https://github.com/dart-lang/native/issues/2548.

Extension types

Back when we designed JNIgen, that was the only way to support generics because we did not have extension types. Extension types allow us to achieve the effect of T.fromReference using JObject.fromReference(...) as T because T is exactly the same as JObject at runtime:

class JObject { /* ... */ }
extension type JInteger(JObject o) implements JObject { /* ... */ }
extension type JString(JObject o) implements JObject { /* ... */ }
extension type JList<T extends JObject>(JObject o) implements JObject {
  T get(int index) {
    final result = ...;
    return result as T; // Fine!
  }
}

Reconsidering our philosophy

If we decide to change the architecture of the JNIgen generated bindings to use extension types instead of the classes + type classes we lose some key benefits. Namely we can't have JList<T> mix in ListMixin<T> so we can't just iterate over a JList as we would now.

However we can't do a lot of things even now:

  • We can't just pass a JString to a method that expects a String because we can't do JString implements String. This means that every time we have to do jstr.toDartString().
  • We can't pass a JInteger to a method that expects an int.
  • We don't generate Java enums as Dart enums because if we want to pass them to other Java methods we need those objects to extend JObject. This means that we can't use a switch over a Java enum in Dart: https://github.com/dart-lang/native/issues/1903
  • ...

Maybe the solution is not trying our best to fit Java and Kotlin into a Dart class structure. But rather create conversion methods that either convert the object completely to a Dart object or convert it to a "wrapper" type of object that is more convenient to work with.

This means that we can only do this conversion one layer at a time. We can convert a JList<JList<T>> into a List<JList<T>> and not a List<List<T>>. So if we want to do a nested loop on this list we would do:

for (final innerList in jlist2d.asDartList()) {
  for (final element in innerList.asDartList()) {
    // ...
  }
}

Currently we don't need this asDartList in our bindings but this comes with a lot of added complexity for the code generator, added runtime due to dynamic dispatch in Dart and running lowest common ancestor to get the right type class in type class inference. All of this goes away if we add another layer to JList as we already do for JString and enums.

It's important to note that tools that do the complete deep conversion will remain functioning as before. If pigeon wants to expose JString as String and JList as List they're free to do so.

User transformations

This means that user transformations gets much smaller in scope. We're not really planning on customizing a class. Rather, any transformation is really done at the wrapper level.

Interop between bindings

All of the generated bindings are of the exact same runtime type of JObject, so even if the classes are not compatible between two libraries, we could cast them using a simple as as a make shift solution.

Prior art

JS interop is already using extension types: https://dart.dev/interop/js-interop/js-types and users use toJS... and toDart... to convert between the types.

Why now?

It's important to decide our path forward before we make JNIgen stable.

@dcharkes @liamappelbe @goderbauer @stuartmorgan-g Let me know your thoughts.

HosseinYousefi avatar Aug 28 '25 14:08 HosseinYousefi

Some notes from our multiple discussions around this.

Performance

For performant interop, extension types are preferred. The new js-interop uses this. And if we would design dart:ffi today, we would use extension types for Pointer, user-defined Structs etc.

Extension types have no runtime representation, so zero cost.

Most notably, extension types have no dynamic dispatch. Dynamic dispatch is already happening in the JVM, so we don't need to also do it on the Dart side.

Layering, single responsibility per layer

Multiple layers, different responsibility per layer:

  • When interoperating with C, there is no object-oriented API at all. Hence, the user will want to write a Dart API around the generated bindings. This is then where this mapping is provided.
  • In js-interop, there are convenience methods JSList.toDart().
    • Note that these convenience methods cannot deeply convert JSList<JSList<JSInteger>>, toDart() returns List<JSList<JSInteger>>.
      • It is exactly this T.fromReference which we are missing. And this is exactly what we have this tree of type objects for.
  • In this logic of multiple layers, we could reason about a jnigen_core and jnigen_fancy where we would generate those two layers explicitly instead of having them conflated to one.
    • In the C interop we don't have this info, so the fancy layer must be handwritten. However, in Java interop we do have this info.
    • The jnigen_fancy generated code should not leak any of the jnigen_core objects, so it has to rewrap all JObjects and all method calls on such objects, and it would keep this type hierarchy.
    • I'm not saying we should do jnigen_fancy, but it serves as a thought-exercise.

The responsibilities per layer:

  • The core layer: calling (and getting callbacks) from the other language with minimal overhead.
  • The Dart adapter layer: adapters that implement Dart interfaces.

Impact on user-experience

I'd be curious what the impact is on the user-experience between:

  1. The current approach, partial implementing Dart classes (but not String, enum, int). (Either achieved via the current approach, or by a jnigen_fancy.)
  2. The jnigen_core approach with explicit conversions between the two layers via toDart() and toJni() calls.

Would it be feasible to get a prototype of jnigen_core and a migration of https://github.com/flutter/packages/pull/9770 (or another real use case) to such prototype?

Also cc @brianquinlan

dcharkes avatar Aug 28 '25 15:08 dcharkes

Would it be feasible to get a prototype of jnigen_core and a migration of flutter/packages#9770 (or another real use case) to such prototype?

I could spend ~two weeks prototyping this and regenerate cronet to see how the code changes.

HosseinYousefi avatar Aug 28 '25 16:08 HosseinYousefi

This means that we can only do this conversion one layer at a time. We can convert a JList<JList<T>> into a List<JList<T>> and not a List<List<T>>. So if we want to do a nested loop on this list we would do:

for (final innerList in jlist2d.asDartList()) {
  for (final element in innerList.asDartList()) {
    // ...
  }
}

Aside: if JList doesn't implement List anymore, we should add a viewer object, JListView or JListDartView or something, that does implement List. That nested loop would become:

for (final innerList in jlist2d.asDartListView()) {
  for (final element in innerList.asDartListView()) {
    // ...
  }
}

The point of the viewer object would be to support iteration etc without converting the entire list to Dart. For large lists, converting the list every time you want to iterate could be expensive.

liamappelbe avatar Aug 29 '25 02:08 liamappelbe

The point of the viewer object would be to support iteration etc without converting the entire list to Dart. For large lists, converting the list every time you want to iterate could be expensive.

I think the goal should be to have adapters that wrap the Java objects rather than deep convert. Though, maybe if you want to pass something to Java you might want to deep convert your Dart data structure. So maybe we should have some kind of naming convention that distinguishes between shallow and deep conversion.

extension on JList<T> {
  // Shallow adapter that holds on to Java object.
  List<T> asDartView();

  // Deep conversion.
  List<S> toDart(S Function(T) elementConverter);
}

extension on List<S> {
  // Deep conversion to JVM.
  JList<T> toJni(T Function(S) elementConverter);

  // Shallow adapter that forwards all Java methods to backing Dart object via sync callbacks using interface implementation.
  //
  // We'll need the element converter here as well if we want to implement the right Java interface. The element converter is called lazily.
  //
  // Though, we could have JList<JObject> alternatively.
  JList<T> asJavaAdapter(T Function(S) elementConverter);
}

Naming TBD.

cc @tarrinneal who is the resident expert in deep conversion.

dcharkes avatar Aug 29 '25 06:08 dcharkes

Aside: if JList doesn't implement List anymore, we should add a viewer object, JListView or JListDartView or something, that does implement List. That nested loop would become:

Yeah that's what I meant. And that's the difference between methods starting with as vs to, the exact naming could be decided.

HosseinYousefi avatar Aug 29 '25 08:08 HosseinYousefi

Would it be feasible to get a prototype of jnigen_core and a migration of flutter/packages#9770 (or another real use case) to such prototype?

I could spend ~two weeks prototyping this and regenerate cronet to see how the code changes.

Sorry for the slow response here; I haven't used extension types, so I needed to ramp up on them.

Given a prototype of an extension-type jnigen, I can try reworking my prototypes to use them, and compare them as a client. I don't think I'll be able to offer an informed opinion without that.

That said, I do generally think a layered approach is good. If we can make a lower-cost core interop, and then make convenience methods an optional higher layer, that seems like a good direction for long-term flexibility and mainetance.

stuartmorgan-g avatar Sep 11 '25 19:09 stuartmorgan-g

I think the goal should be to have adapters that wrap the Java objects rather than deep convert. Though, maybe if you want to pass something to Java you might want to deep convert your Dart data structure.

We will absolutely want (optional, but available at some layer) deep converters in both directions; I've already run into a bunch of cases where I need them.

For instance, needing to wrap a Dart API around a Java call that returns a list of strings (or an object that includes one) is something I've hit several times. I'm not going to return a JList<JString> or a List<JString> out of my Dart-friendly API, and every client of jnigen shouldn't have to re-invent that wheel.

stuartmorgan-g avatar Sep 11 '25 20:09 stuartmorgan-g

I'm not going to return a JList<JString> or a List<JString> out of my Dart-friendly API, and every client of jnigen shouldn't have to re-invent that wheel.

Well, Pigeon has the feature for you! (soon™)

tarrinneal avatar Sep 11 '25 22:09 tarrinneal

Given a prototype of an extension-type jnigen, I can try reworking my prototypes to use them, and compare them as a client. I don't think I'll be able to offer an informed opinion without that.

Alright then, I'll start working on it today and update you once it's available on a branch.

We will absolutely want (optional, but available at some layer) deep converters in both directions; I've already run into a bunch of cases where I need them.

Agreed.

HosseinYousefi avatar Sep 12 '25 13:09 HosseinYousefi

@HosseinYousefi Can you give a quick update on this? Are you definitely going to go ahead with this change, or are there unforeseen problems? If I'm going to do the same thing for ffigen, I'd like to land it before the mid-Nov release, so I should start working on it soon.

liamappelbe avatar Oct 01 '25 23:10 liamappelbe

@HosseinYousefi Can you give a quick update on this? Are you definitely going to go ahead with this change, or are there unforeseen problems? If I'm going to do the same thing for ffigen, I'd like to land it before the mid-Nov release, so I should start working on it soon.

I have not had serious issues with this yet. I'm OOO this week but I'll finish the migration next week and report back.

HosseinYousefi avatar Oct 01 '25 23:10 HosseinYousefi