swiftlang / swift-java

Apache License 2.0
735 stars 29 forks source link

JavaKit: Java maintaining references to Swift values without Panama #168

Open lhoward opened 3 weeks ago

lhoward commented 3 weeks ago

Edit: something I have found useful. Refactored to not use finalize(), and added links. Essentially:

import java.lang.ref.Cleaner;

class SwiftHeapObjectHolder implements AutoCloseable {
  private static final Cleaner cleaner = Cleaner.create();

  private final Cleaner.Cleanable _cleanable;
  public long _swiftHeapObject;

  public SwiftHeapObjectHolder(long heapObjectInt64Ptr) {
    final Runnable F = () -> SwiftHeapObjectHolder._releaseSwiftHeapObject(heapObjectInt64Ptr);
    SwiftHeapObjectHolder._retainSwiftHeapObject(heapObjectInt64Ptr);
    _cleanable = cleaner.register(this, F);
    _swiftHeapObject = heapObjectInt64Ptr;
  }

  @Override
  public void close() throws Exception {
    _swiftHeapObject = 0;
    _cleanable.clean();
  }

  static native void _retainSwiftHeapObject(long heapObjectInt64Ptr);
  static native void _releaseSwiftHeapObject(long heapObjectInt64Ptr);
}

https://github.com/PADL/FlutterSwift/blob/main/Sources/FlutterAndroid/com/padl/FlutterAndroid/SwiftHeapObjectHolder.java https://github.com/PADL/FlutterSwift/blob/main/Sources/FlutterAndroid/SwiftHeapObjectHolderNativeMethods.swift

ktoso commented 3 weeks ago

No doubt you don't want to ship JARs.

? We'll absolutely be shipping some libraries or plugins as jars eventually.

ktoso commented 3 weeks ago

We should not be using finalizers if we can help it.

Please have a look at SwiftKit and the SwiftArena, this is the way to manage swift resources in Java, it's a more predictable modern way of doing so. There is an AutoArena which uses phantom references to release when references are dropped in a more efficient way.

lhoward commented 3 weeks ago

Ah, just thought that would make the build process even more of a pain the proverbial!

ktoso commented 3 weeks ago

Can we rephrase this ticket in terms of the problem to solve rather than propose the solution?

The "modern way" is SwiftArena and we should look into using it first if this is about any plain old "keep object alive while i have references to it in java".

lhoward commented 3 weeks ago

Sure. I wasn't aware of SwiftArena. But it looks like it requires Panorama, whereas Android I believe is still at JDK 17. Will confirm shortly.

ktoso commented 3 weeks ago

So funnily enough SwiftArena is not a JDK Arena type but its usage is very similar (but we never use it to "allocate"), the AutoSwiftArena just uses phantom references which have been around in the JDK for long.

I think we should pursue making that lib have some parts which are usable on older platforms as well, and that may include the AutoSwiftArena perhaps - then we don't have to invent another new way to solve the same issue 🤔 Maybe we'd do some swiftkit-core that's compatible with older platforms

ktoso commented 3 weeks ago

Thanks for the rename! :)

lhoward commented 3 weeks ago

Sounds good. I may not have the cycles to help on this but I'll have a think!

lhoward commented 3 weeks ago

Pleasure. Here I was thinking I had thought of something you hadn't.

ktoso commented 3 weeks ago

I'm sure you'll bump into a lot of things we've not thought about yet, this one I had a sneaky plan for though :)

lhoward commented 3 weeks ago

The biggest remaining issue for me is object/JNIEnv affinity, but there's an open issue for that 😉

lhoward commented 3 weeks ago

We should not be using finalizers if we can help it.

Please have a look at SwiftKit and the SwiftArena, this is the way to manage swift resources in Java, it's a more predictable modern way of doing so. There is an AutoArena which uses phantom references to release when references are dropped in a more efficient way.

Amusingly SwiftKit uses _typeByName() which I proposed nearly ten years ago. Must be getting old.

ktoso commented 3 weeks ago

We will actually stop using that as soon as I finish my “extract build plugin” PR, but yeah it’s still hidden api…

lhoward commented 3 weeks ago

Of potential note: https://github.com/vova7878/PanamaPort

ktoso commented 3 weeks ago

Oh that's very interesting, perhaps a route we could attempt huh 🤔 Thanks for sharing!

lhoward commented 2 weeks ago

I think I will just migrate my code to use Cleanable instead of finalize(). I don't have the time right now to unpick the back-portable bits of SwiftArena.

ktoso commented 2 weeks ago

Yeah that would already be a nice step! And that’ll then be easy to hook into the arena :) I can look at these later on

lhoward commented 2 weeks ago

Yeah that would already be a nice step! And that’ll then be easy to hook into the arena :) I can look at these later on

Pasted in code/links to (edited) original post in case you feel like taking a look. Perhaps I should add C JNI wrappers for swift_retain and swift_release rather than using Unmanaged.

ktoso commented 2 weeks ago

Yeah that's right.

This is done by https://github.com/swiftlang/swift-java/blob/main/SwiftKit/src/main/java/org/swift/swiftkit/AutoSwiftMemorySession.java#L47-L51 already, no need to reinvent -- just reorganize the code so this part of it lives in a non-FFM dependent part of the project. We can make some SwiftKitBasic maybe, and the destroy Runnables actually doing FFM invocations into native we'd then have in a separate module that does use FFM. but we can have others -- like this one just reference counting or using JNI for the destroy somehow?

I think we should look into sharing the infrastructure basically

lhoward commented 2 weeks ago

Sounds good. Won't be able to look at this again for a few weeks.