Skip to content

Separate SwiftKit into SwiftKitCore and SwiftKitFFM #300

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

Merged
merged 9 commits into from
Jul 8, 2025

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

@@ -27,9 +27,10 @@ fi

# check if we can compile a plain Example file that uses the generated Java bindings that should be in the generated jar
# 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 :)

@@ -148,7 +147,8 @@ tasks.clean {
}

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