Initial commit of Okio ZStd by swankjesse · Pull Request #1 · square/zstd-kmp

swankjesse · 2025-07-11T14:39:31Z

This initial commit supports only the JVM. But it is structured to make it straightforward to support
Kotlin/Multiplatform.

Build infrastructure is all derived from Zipline.

This initial commit supports only the JVM. But it is structured to make it straightforward to support Kotlin/Multiplatform. Build infrastructure is all derived from Zipline.

swankjesse · 2025-07-11T14:40:22Z

I already ran a benchmark on this and all of my dreams have come true.

Benchmark                     (algorithm)  (sampleDataSize)  Mode  Cnt   Score    Error  Units
CompressBenchmark.compress        deflate           8388608  avgt    3  86.539 ± 14.696  ms/op
CompressBenchmark.compress           gzip           8388608  avgt    3  84.996 ± 20.841  ms/op
CompressBenchmark.compress           zstd           8388608  avgt    3  13.248 ±  1.842  ms/op
CompressBenchmark.compress           none           8388608  avgt    3   0.401 ±  0.052  ms/op
CompressBenchmark.decompress      deflate           8388608  avgt    3  11.126 ±  1.089  ms/op
CompressBenchmark.decompress         gzip           8388608  avgt    3   9.761 ±  0.920  ms/op
CompressBenchmark.decompress         zstd           8388608  avgt    3   4.339 ±  0.562  ms/op
CompressBenchmark.decompress         none           8388608  avgt    3   0.407 ±  0.023  ms/op

swankjesse · 2025-07-11T14:42:28Z

okio-zstd/api/okio-zstd.api

@@ -0,0 +1,5 @@
+public final class okio/zstd/Zstd {


Very minimal API to start. I think we might later want to expose a mechanism to configure the compressor, but that’s not urgent at all.

.github/workflows/build.yaml

.github/workflows/gradle-wrapper.yaml

.github/workflows/release.yaml

swankjesse · 2025-07-11T16:05:46Z

build.gradle.kts

+
+buildscript {
+  repositories {
+    mavenCentral()


The Gradle Repositories configuration was identifed as containing references to an external package registry. Block requires that we only pull down Artifacts from our internal registries and/or mirrors. All references to mavenCentral(), google(), jcenter() or ivy() must be removed

I don’t think this rule holds for open source projects.

swankjesse · 2025-07-11T16:05:32Z

build.gradle.kts

+  version = project.property("VERSION_NAME") as String
+
+  repositories {
+    mavenCentral()


The Gradle Repositories configuration was identifed as containing references to an external package registry. Block requires that we only pull down Artifacts from our internal registries and/or mirrors. All references to mavenCentral(), google(), jcenter() or ivy() must be removed

I don’t think this rule holds for open source projects.

swankjesse · 2025-07-11T14:42:59Z

okio-zstd/build.zig

+  lib.addIncludePath(b.path("../zstd/lib"));
+
+  lib.linkLibC();
+  // TODO Tree-walk these two dirs for all C files.


This is the part of the PR where I hold the most shame. I just have to learn how to Zig instead of copy-pasting

swankjesse · 2025-07-11T14:44:07Z

okio-zstd/native/OkioZstd.cpp

+}
+
+extern "C" JNIEXPORT jlong JNICALL
+Java_okio_zstd_JniZstdCompressor_compressStream2(JNIEnv* env, jobject type, jlong jniZstdPointer, jlong cctxPointer, jobject output, jobject input, jint mode) {


The 2 here is 'cause that’s the Zstd function I’m calling. I’m tempted to rename to just compress

swankjesse · 2025-07-11T14:44:59Z

okio-zstd/src/commonMain/kotlin/okio/zstd/Zstd.kt

+internal expect fun zstdCompressor(): ZstdCompressor
+
+/** Returns a new decompressor. The caller must close it. */
+internal expect fun zstdDecompressor(): ZstdDecompressor


These are the three 3 functions to implement on Kotlin/Native !

swankjesse · 2025-07-11T14:46:12Z

okio-zstd/src/commonMain/kotlin/okio/zstd/ZstdCompressSink.kt

+        ZSTD_e_continue -> inputRemaining == 0L
+        else -> inputRemaining == 0L && result == 0L
+      }
+    } while (!finished)


This whole loop should approximately mirror the Zstd sample:
https://github.com/facebook/zstd/blob/dev/examples/multiple_streaming_compression.c#L80

swankjesse · 2025-07-11T14:46:59Z

okio-zstd/src/commonMain/kotlin/okio/zstd/ZstdDecompressSource.kt

+
+      lastDecompressResult = result
+      result.checkError()
+    }


This whole loop should track another Zstd sample:

https://github.com/facebook/zstd/blob/dev/examples/streaming_decompression.c#L39

swankjesse · 2025-07-11T14:48:39Z

okio-zstd/src/jvmTest/kotlin/okio/zstd/ZstdDecompressSourceTest.kt

+import okio.buffer
+import okio.sink
+
+class ZstdDecompressSourceTest {


These tests are currently in jvmTest and not commonTest cause I’m confirming interop with com.github.luben.zstd.ZstdOutputStream.

I wanna add some more golden file tests, and then change these to interop with our own compressor. (And vice-versa for the decompression test.)

Is this more secure? https://github.com/sethvargo/ratchet https://github.com/square/okio-zstd/security/code-scanning/1

yschimke

LGTM - might be a fun project for a fuzzer

yschimke · 2025-07-12T09:43:05Z

.github/renovate.json5

README.md

yschimke · 2025-07-12T09:52:07Z

okio-zstd/native/OkioZstd.cpp

+ *
+ * Pass a pointer to this to all JNI functions that operate on JVM objects.
+ */
+class JniZstd {


I'm not much use on this part of the review.

okio-zstd/src/commonMain/kotlin/okio/zstd/Zstd.kt

yschimke · 2025-07-12T10:04:59Z

okio-zstd/src/commonMain/kotlin/okio/zstd/ZstdCompressSink.kt

+  override fun write(source: Buffer, byteCount: Long) {
+    check(!closed) { "closed" }
+
+    inputBuffer.write(source, byteCount)


If this fails, do you still expect the caller to be a boy scout and clean up by calling close?

Since this is native code, should this have some fallback like a Cleaner task if caller doesn't release the native resources?

Nothing for now. I’m mixed on whether to build that in.

I feel like native resources you need to. Not cleaning up java classes eventually gets GCed. But the native code becomes a slow process death.

I guess there is leakcanary etc, to show unreleased resources.

okio-zstd/src/jvmMain/kotlin/okio/zstd/JniZstd.kt

Initial commit of Okio ZStd

e20f479

This initial commit supports only the JVM. But it is structured to make it straightforward to support Kotlin/Multiplatform. Build infrastructure is all derived from Zipline.

swankjesse commented Jul 11, 2025

View reviewed changes

github-advanced-security bot found potential problems Jul 11, 2025

View reviewed changes

swankjesse commented Jul 11, 2025

View reviewed changes

Use ratchet to pin actions dependencies

d99ab28

Is this more secure? https://github.com/sethvargo/ratchet https://github.com/square/okio-zstd/security/code-scanning/1

swankjesse requested a review from yschimke July 11, 2025 21:24

yschimke approved these changes Jul 12, 2025

View reviewed changes

swankjesse merged commit 6fe3fa9 into main Jul 12, 2025
6 checks passed

swankjesse deleted the jwilson.0711.zstd branch July 12, 2025 20:31

Conversation

swankjesse commented Jul 11, 2025

Uh oh!

swankjesse commented Jul 11, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Check failure

Choose a reason for hiding this comment

Uh oh!

Check failure

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!

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!

yschimke left a comment

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!

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!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants