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

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

Open
jminor opened this issue Feb 12, 2024 · 5 comments
Open

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

jminor opened this issue Feb 12, 2024 · 5 comments

Comments

@jminor
Copy link
Member

jminor commented Feb 12, 2024

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: AcademySoftwareFoundation/tac#578 including @furby-tm's slides which give an overview: Swift_Working_Group.pdf

@meshula
Copy link
Member

meshula commented Feb 13, 2024

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
Copy link

furby-tm commented Feb 13, 2024

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
Copy link
Member

meshula commented Feb 13, 2024

Thanks for the notes!

@furby-tm
Copy link

furby-tm commented Feb 14, 2024

@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
Copy link

furby-tm commented Mar 16, 2024

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants