C library for Pkl #1026

KushalP · 2025-03-18T17:51:56Z

This introduces native C bindings for Pkl.

Dynamic Library

Using org.graalvm.nativeimage and the native-image binary we
produce a dynamic library (for each OS/Arch variant) that provides a
way to initialise itself with a graal_isolatethread_t.

Methods are annotated with @CEntryPoint and exported.

This change results in an architecture and OS-specific directory being
created which now produces the headers for our shared library
functions:

❯ ll libpkl/build/libs/macos-aarch64/

graal_isolate_dynamic.h
graal_isolate.h
libpkl_internal_dynamic.h
libpkl_internal.dylib
libpkl_internal.h

`libpkl`

The produced libpkl dynamic library wraps the GraalVM C interface
into something that is future-friendly for the needs of a Pkl
integrator. It exports an interface which aligns with SPICE-0015[1].

❯ ll libpkl/build/libs/macos-aarch64/

graal_isolate_dynamic.h
graal_isolate.h
libpkl_internal_dynamic.h
libpkl_internal.dylib
libpkl_internal.h
libpkl.dylib    <--- this is new
pkl.h           <--- this is new

JNA

Testing of the produced libpkl dynamic library is done using Java
Native Access[2] for ease. We provide an interface in Kotlin which
JNA transposes against the libpkl dynamic library discoverable at
the path that is discoverable with jna.library.path.

Load in `projects.pklCommonsCli` to deal with `UnsupportedFeatureException`

This is to deal with the following error:

Caused by: com.oracle.graal.pointsto.constraints.UnsupportedFeatureException: Detected a started Thread in the image heap. Thread name: main. Threads running in the image generator are no longer running at image runtime. If these objects should not be stored in the image heap, you can use

    '--trace-object-instantiation=java.lang.Thread'

[1] apple/pkl-evolution#16

libpkl/src/main/c/libpkl.h

KushalP · 2025-04-25T08:49:27Z

libpkl/src/main/java/org/pkl/libpkl/LibPklLogger.java

+    if (Objects.equals(System.getenv("PKL_DEBUG"), "1")) {
+      System.err.println("[libpkl] " + msg);
+    }


Does this work? What's the preferred approach to take here?

You should be able to test this locally (set the env var, run unit tests and see what happens)

libpkl/src/main/java/org/pkl/libpkl/NativeTransport.java

libpkl/src/test/kotlin/org/pkl/libpkl/ILibPklLibrary.kt

bioball

Great work so far! I left some comments, but it's great to see this already working.

bioball · 2025-04-25T16:01:41Z

libpkl/src/main/c/libpkl.h

+typedef void (*PklMessageResponseHandler)(int length, char* message);
+
+int pkl_init(PklMessageResponseHandler handler);
+
+int pkl_send_message(int length, char* message);
+
+int pkl_close();


Can you add doxygen-style doc comments to these methods?

Also: let's add a void pointer here (per our convos with some C folks); we'd just keep a reference to this pointer and pass it back when calling PklMessageResponseHandler.

Suggested change

typedef void (*PklMessageResponseHandler)(int length, char* message);

int pkl_init(PklMessageResponseHandler handler);

int pkl_send_message(int length, char* message);

int pkl_close();

typedef void (*PklMessageResponseHandler)(int length, char *message, void *handlerContext);

int pkl_init(PklMessageResponseHandler handler, void *handlerContext);

int pkl_send_message(int length, char *message);

int pkl_close();

BTW: it seems like putting the asterisk next to the name of the variable is more idiomatic in C

How would you expect handlerContext to be provided back to the user in int pkl_init(PklMessageResponseHandler handler, void *handlerContext); ?

We'd just pass it back every time we call handler, which also accepts void *handlerContext.

libpkl/src/main/java/org/pkl/libpkl/LibPkl.java

bioball · 2025-04-25T16:50:22Z

libpkl/src/main/c/libpkl.c

+#include <pthread.h>
+
+#include <graal_isolate.h>
+#include <libpkl-internal-macos-aarch64.h>


We don't need to address this before merging into a feature branch, but, this file should be platform-independent.

I think this should be:

#include <pkl_internal.h>

The platform-specific header/library would then be provided as a linker option.

This is almost there. I don't know if there's a way to get native-image to produce a dynamic library prefixed with lib whilst the .h file it creates doesn't have the lib prefix. Otherwise we'll have to rename it ourselves.

It's not a big deal if it does have a lib prefix, I guess. That's an internal file anyway; users would import pkl.h and not libpkl_internal.h

libpkl/src/main/java/org/pkl/libpkl/LibPkl.java

bioball · 2025-04-25T17:48:26Z

libpkl/src/main/java/org/pkl/libpkl/LibPkl.java

+  private static MessageCallbackFunctionPointer cb;
+
+  @CEntryPoint(name = "pkl_internal_init", builtin = CEntryPoint.Builtin.CREATE_ISOLATE)
+  static native IsolateThread pklInternalInit();


Is there a particular reason we are creating our own method, instead of graal_create_isolate from the Native Image C API?

https://www.graalvm.org/latest/reference-manual/native-image/native-code-interoperability/C-API/

I think we can just handle this in our pkl_init:

graal_isolatethread_t *isolatethread = NULL; graal_isolate_t *isolate = NULL; int pkl_init(PklMessageResponseHandler handler) { // etc if (graal_create_isolate(NULL, &isolate, &isolatethread) != 0) { return -1 }

I thought it would be consistent with how all of the pkl-internal bits are exposed through LibPkl.java. This would now be one case where the management of Graal-specific resources is handled out-of-band in the .c file. If you'd prefer me to do the above let me know.

I'm not sure what the benefit of doing it this way is, especially since there already is a C method that does the same thing.

But, they have a tutorial where they do it this way too: https://www.graalvm.org/21.3/reference-manual/native-image/ImplementingNativeMethodsInJavaWithSVM/

So, I guess let's just keep this? We can ask about this in their Slack too.

bioball · 2025-04-25T18:44:43Z

libpkl/src/test/kotlin/org/pkl/libpkl/JNATest.kt

+    val unpacker = MessagePack.newDefaultUnpacker(response.result)
+    val value = unpacker.unpackValue()
+    assertThat(value.isArrayValue)
+  }


We can address this in a follow-up PR, but: I think let's move org.pkl.server.AbstractServerTest into pkl-commons-test.

libpkl/src/main/java/org/pkl/libpkl/LibPkl.java

bioball · 2025-04-25T20:23:42Z

libpkl/src/test/kotlin/org/pkl/libpkl/JNATestClient.kt

+import org.pkl.server.ServerMessagePackDecoder
+import org.pkl.server.ServerMessagePackEncoder
+
+class JNATestClient : ILibPklLibrary.PklMessageResponseHandler, Iterable<Message?>, AutoCloseable {


This class is mixing quite a few concerns.

Does this need to implement Iterable? I don't see where that's being used

Should separate the callback into a different object

I think this can just have an init() that calls into ILibPklLibrary:

private val messageResponseHandler: PklMessageResponseHandler = object : PklMessageResponseHandler { override fun invoke(length: Int, message: Pointer) { val receivedBytes = message.getByteArray(0, length) val message = decode(receivedBytes) assertThat(message).isNotNull incoming.add(message!!) } } fun init() { assertThat(ILibPklLibrary.INSTANCE.pkl_init(messageResponseHandler)).isEqualTo(0) } override fun close() { incoming.clear() assertThat(ILibPklLibrary.INSTANCE.pkl_close()).isEqualTo(0) }

Does this need to implement Iterable? I don't see where that's being used

By making it Iterable in JNATest.kt I can write assertions like the following: assertThat(client).hasSize(1).

libpkl/src/main/java/org/pkl/libpkl/LibPkl.java

bioball · 2025-05-07T21:37:43Z

libpkl/src/main/c/pkl.h

+* @return -1 on failure.
+* @return 0 on success.
+*/
+int pkl_send_message(int length, char *message, void *handlerContext);


void *handlerContext should be on pkl_init, not pkl_send_message.

This introduces native C bindings for Pkl. Dynamic Library --------------- Using `org.graalvm.nativeimage` and the `native-image` binary we produce a dynamic library (for each OS/Arch variant) that provides a way to initialise itself with a `graal_isolatethread_t`. Methods are annotated with `@CEntryPoint` and exported. This change results in an architecture and OS-specific directory being created which now produces the headers for our shared library functions: ``` ❯ ll libpkl/build/libs/macos-aarch64/ graal_isolate_dynamic.h graal_isolate.h libpkl-internal_dynamic.h libpkl-internal-macos-aarch64_dynamic.h libpkl-internal-macos-aarch64.dylib libpkl-internal-macos-aarch64.h libpkl-internal.dylib libpkl-internal.h ``` `libpkl` -------- The produced `libpkl` dynamic library wraps the GraalVM C interface into something that is future-friendly for the needs of a Pkl integrator. It exports an interface which aligns with SPICE-0015[1]. ``` ❯ ll libpkl/build/libs/macos-aarch64/ graal_isolate_dynamic.h graal_isolate.h libpkl-internal_dynamic.h libpkl-internal-macos-aarch64_dynamic.h libpkl-internal-macos-aarch64.dylib libpkl-internal-macos-aarch64.h libpkl-internal.dylib libpkl-internal.h libpkl.dylib <--- this is new libpkl.h <--- this is new ``` JNA --- Testing of the produced `libpkl` dynamic library is done using Java Native Access[2] for ease. We provide an `interface` in Kotlin which JNA transposes against the `libpkl` dynamic library discoverable at the path that is discoverable with `jna.library.path`. Load in `projects.pklCommonsCli` to deal with `UnsupportedFeatureException` --------------------------------------------------------------------------- This is to deal with the following error: ``` Caused by: com.oracle.graal.pointsto.constraints.UnsupportedFeatureException: Detected a started Thread in the image heap. Thread name: main. Threads running in the image generator are no longer running at image runtime. If these objects should not be stored in the image heap, you can use '--trace-object-instantiation=java.lang.Thread' ``` [1] apple/pkl-evolution#16

This allows a user to provide a pointer, and have it be passed through and back to their `PklMessageResponseHandler` handler for them track as part of their own bookkeeping.

KushalP force-pushed the c-library-for-pkl branch 7 times, most recently from 80a0c54 to 3e26bbb Compare March 25, 2025 14:26

KushalP force-pushed the c-library-for-pkl branch 2 times, most recently from 1226e49 to 4d07f34 Compare April 11, 2025 10:11

KushalP force-pushed the c-library-for-pkl branch 2 times, most recently from 01d886a to a34e1ce Compare April 17, 2025 14:32

KushalP force-pushed the c-library-for-pkl branch 3 times, most recently from ec1199c to 3db68c1 Compare April 24, 2025 12:52

KushalP commented Apr 25, 2025

View reviewed changes

libpkl/src/main/c/libpkl.h Outdated Show resolved Hide resolved

KushalP commented Apr 25, 2025

View reviewed changes

libpkl/src/main/java/org/pkl/libpkl/NativeTransport.java Outdated Show resolved Hide resolved

KushalP commented Apr 25, 2025

View reviewed changes

libpkl/src/main/java/org/pkl/libpkl/NativeTransport.java Outdated Show resolved Hide resolved

bioball reviewed Apr 25, 2025

View reviewed changes

libpkl/src/test/kotlin/org/pkl/libpkl/ILibPklLibrary.kt Outdated Show resolved Hide resolved

bioball reviewed Apr 25, 2025

View reviewed changes

KushalP force-pushed the c-library-for-pkl branch from 3db68c1 to 5acc28d Compare May 7, 2025 10:31

bioball changed the base branch from main to c-library May 7, 2025 21:17

bioball reviewed May 7, 2025

View reviewed changes

KushalP force-pushed the c-library-for-pkl branch 2 times, most recently from cc16920 to e3598b0 Compare May 27, 2025 09:13

bioball marked this pull request as ready for review May 28, 2025 21:13

KushalP added 4 commits May 28, 2025 14:13

Rename to LibPklLibrary

dc85b52

Remove lib prefix from .h and .c files

6876907

Add doxygen-style comments to pkl.h file

5a9e685

KushalP and others added 10 commits May 28, 2025 14:13

Move pointer next to variable name

e44e441

Add TODO comment to clean this up once on a feature-branch

f0c4176

Add @SuppressWarnings("unused") annotation to LibPkl class

b09abff

Move LibPkl constructor between fields and methods

6102e45

Add package-info.java for org.pkl.libpkl package

1996964

Propagate param handlerContext so users can trace messages

30c1a46

This allows a user to provide a pointer, and have it be passed through and back to their `PklMessageResponseHandler` handler for them track as part of their own bookkeeping.

Drop the OS and Arch name from the native-image shared library

ba5d361

TODO: Provide a meaningful error to user if cb == null

b1b1bab

Log exceptions in NativeTransport

0fd8ab3

Address PR comments

e4ca22c

bioball force-pushed the c-library-for-pkl branch from a61fc37 to e4ca22c Compare May 28, 2025 21:13

bioball merged commit 3bf48dd into apple:c-library May 28, 2025
0 of 2 checks passed

C library for Pkl #1026

C library for Pkl #1026

Uh oh!

Conversation

KushalP commented Mar 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Dynamic Library

libpkl

JNA

Load in projects.pklCommonsCli to deal with UnsupportedFeatureException

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bioball left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bioball May 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

KushalP commented Mar 18, 2025 •

edited

Loading

`libpkl`

Load in `projects.pklCommonsCli` to deal with `UnsupportedFeatureException`

bioball May 7, 2025 •

edited

Loading