OpenTimelineIO / OpenTimelineIO-Swift-Bindings

Swift bindings for the OpenTimelineIO Library (http://opentimeline.io)
http://opentimeline.io
Apache License 2.0
17 stars 11 forks source link

Compare current bindings to automatic Swift/C++ interpo #53

Open jminor opened 8 months ago

jminor commented 8 months ago

Swift/C++ interoperation support has come a long way since we last looked. We should re-evaluate and compare to the approach taken in this repo to see which strategy is best.

See also this proposal: https://github.com/AcademySoftwareFoundation/tac/issues/578 including @furby-tm's slides which give an overview: Swift_Working_Group.pdf

meshula commented 8 months ago

I might suggest a few metrics for evaluation. There may be others, but I'll throw these ones out to start ~

  1. Maintainability, of both the delivered code and the infrastructure such as generators
  2. Idiomatic output, identifying such affordances as ranged loops and so on. The current bindings hide much of the shimming and boilerplate result from the next point ~
  3. Correctness of memory management. OTIO is built with Pythonic refcounting and object model in mind, and we have retainer objects as a shim between that and std containers. Swift introduces its own ARC over that, how can we evaluate object lifespan?
  4. Stability of bound interface; will interfaces "swim around" with an automatic generator if the C++ interfaces change?
  5. Concurrency, evaluated against MRMW and MRSW scenarios.
furby-tm commented 8 months ago

I might suggest a few metrics for evaluation. There may be others, but I'll throw these ones out to start ~

  1. Maintainability, of both the delivered code and the infrastructure such as generators

In terms of maintainability we are restructuring our approach to pull from the official source, there are no generators we are using as Swift 5.9 provides this out of the box by enabling interoperabilityMode(.Cxx) on the swift target that imports the CXX module.

OpenTimelineIO is the next library I am working on to bring over, which will hopefully allow you guys to look over the steps we took and give an apt comparison to your existing Swift bindings.

  1. Correctness of memory management. OTIO is built with Pythonic refcounting and object model in mind, and we have retainer objects as a shim between that and std containers. Swift introduces its own ARC over that, how can we evaluate object lifespan?

There is a SWIFT_SHARED_REFERENCE macro which should allow you to evaluate lifespan of an object, the macro works in the following way:

/// Specifies that a C++ `class` or `struct` is reference-counted using
/// the given `retain` and `release` functions. This annotation lets Swift import
/// such a type as reference counted type in Swift, taking advantage of Swift's
/// automatic reference counting.
///
/// This example shows how to use this macro to let Swift know that
/// a non-copyable reference counted C++ class can be imported as a reference counted type in Swift:
///  ```c++
///    class SWIFT_SHARED_REFERENCE(retainSharedObject, releaseSharedObject)
///    SharedObject : NonCopyable, IntrusiveReferenceCounted<SharedObject> {
///    public:
///      static SharedObject* create();
///      void doSomething();
///    };
///
///    void retainSharedObject(SharedObject *);
///    void releaseSharedObject(SharedObject *);
///  ```
///
///  Then, the Swift programmer would be able to use it in the following manner:
///
///  ```swift
///    let object = SharedObject.create()
///    object.doSomething()
///    // The Swift compiler will release object here.
///  ```
#define SWIFT_SHARED_REFERENCE(_retain, _release)                                \
  __attribute__((swift_attr("import_reference")))                          \
  __attribute__((swift_attr(_CXX_INTEROP_STRINGIFY(retain:_retain))))      \
  __attribute__((swift_attr(_CXX_INTEROP_STRINGIFY(release:_release))))

Fortunately the addition of adding clang attributes to existing classes will be simply ignored in all other contexts outside of Swift, as to not introduce any portability issues with its existing implementation, some further documentation regarding swift_attr is documented here.

This comes from a file called <swift/bridging> which was not clear to me if this include was available for Linux platforms, however a measure that I took to bring this header in a cross-platform way is this swiftInterop.h file from the SwiftUSD project.

meshula commented 8 months ago

Thanks for the notes!

furby-tm commented 8 months ago

@meshula quick update! The (initial out-of-the-box automatically generated) bindings are now available from here if you'd like to check them out:

// swift-tools-version: 5.9

dependencies: [
  .package(url: "https://github.com/wabiverse/MetaverseKit", from: "1.5.1")
]

...

.executableTarget(
  name: "MyOTIOApp",
  dependencies: [
    .product(name: "OpenTimelineIO", package: "MetaverseKit"),
  ]
  swiftSettings: [
    .interoperabilityMode(.Cxx)
  ]
)

It however did require moving the headers into include directories for each of the 2 targets as seen in this revision, swift targets do not allow you (or I have not yet figured out how) to share a common root include directory (in OTIO's case /src) for the publicHeadersPath.

furby-tm commented 7 months ago

@meshula @jminor quick update on this:

This comes from a file called <swift/bridging> which was not clear to me if this include was available for Linux platforms, however a measure that I took to bring this header in a cross-platform way is this swiftInterop.h file from the SwiftUSD project.

If you guys are doing something similar - be sure to keep it portable with the "vanilla swift toolchain" if you (or your users) are not using the Xcode toolchain with this addition.

Though in hindsight, you can probably just do away with that include altogether since __has_include() is a Cxx 17 feature anyway, you should be fine to roll without that include, I've only added it if new convenience macros become available later on.