Skip to content

Conversation

@madsodgaard
Copy link
Contributor

@madsodgaard madsodgaard commented Jul 3, 2025

The goal of this PR is to separate SwiftKit into two modules:

  • SwiftKitCore: A module that supports non-FFM memory management and uses JNI for any downcalls.
  • SwiftKitFFM: A module that uses FFM for allocation of memory and downcalls.

@ktoso
Copy link
Collaborator

ktoso commented Jul 7, 2025

But the vision I have is that SwiftInstance now provides a destroy method that the JNI code can then override to call into the VWT from Swift and deallocate the pointer, while FFM can still use the existing VWT calling.

I think that's the right call, but let's avoid getting the method via reflection and instead just give the cleanup something to run, rather that reflection trick and it'll be fine 👍

@madsodgaard madsodgaard changed the title [WIP] Separate SwiftKit into SwiftKitCore and SwiftKitFFM Separate SwiftKit into SwiftKitCore and SwiftKitFFM Jul 8, 2025
@madsodgaard madsodgaard marked this pull request as ready for review July 8, 2025 11:29
@madsodgaard madsodgaard requested a review from ktoso July 8, 2025 11:51
- name: Gradle :SwiftKitFFM:build
run: ./gradlew :SwiftKitFFM:build -x test
- name: Gradle :SwiftKitFFM:check
run: ./gradlew :SwiftKitFFM:check --info
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah that's nice actually to keep them separate rather than one big :check

# The classpath MUST end with a * if it contains jar files, and must not if it directly contains class files.
SWIFTKIT_CLASSPATH="$(pwd)/../../SwiftKit/build/libs/*"
SWIFTKIT_CORE_CLASSPATH="$(pwd)/../../SwiftKitCore/build/libs/*"
SWIFTKIT_FFM_CLASSPATH="$(pwd)/../../SwiftKitFFM/build/libs/*"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, sorry you had to go through figuring that out, but now you know hah :)

dependencies {
implementation(project(':SwiftKit'))
implementation(project(':SwiftKitCore'))
implementation(project(':SwiftKitFFM'))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick, no need to fix up right now -- since FFM depends on Core we could just list the FFM dependency here, it'd resolve properly I think even with project dependencies


@Override
public void run() {
markAsDestroyed.run();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can do this in a follow up but we should only run the destroy if the mark permits us to do so; we can fix that later though

Copy link
Collaborator

@ktoso ktoso left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good, let's get that in :)

@ktoso ktoso merged commit d266a44 into swiftlang:main Jul 8, 2025
28 checks passed
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

Successfully merging this pull request may close these issues.

2 participants