Skip to content

Improve zstd performance & fix KTOR-9487#5519

Open
solonovamax wants to merge 3 commits intoktorio:mainfrom
solonovamax:feature/zstd-performance
Open

Improve zstd performance & fix KTOR-9487#5519
solonovamax wants to merge 3 commits intoktorio:mainfrom
solonovamax:feature/zstd-performance

Conversation

@solonovamax
Copy link
Copy Markdown
Contributor

Subsystem
shared/ktor-encoding-zstd

Motivation
I didn't like the way it did things so I took a deeper look at it and cleaned it up.
in the process I also discovered KTOR-9487.

Solution
This should improve zstd encoding & decoding performance because it avoids allocating byte arrays on the heap, instead relying on the byte buffers.

Also fixes KTOR-9487 by just using the streaming api like it should have from the start.
I also fixed the test which wasn't actually correctly testing what it said it was testing.

I had to add a new byte buffer pool because the zstd encoder needs direct byte buffers.

This should improve zstd encoding & decoding performance
because it avoids allocating byte arrays on the heap,
instead relying on the byte buffers.

This also fixes KTOR-9487 by just using the streaming api like it should have from the start.

Signed-off-by: solonovamax <solonovamax@12oclockpoint.com>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 12, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: c6ef9153-4563-4214-b2de-fe3889f49b96

📥 Commits

Reviewing files that changed from the base of the PR and between 9f3bb30 and 8ed728e.

📒 Files selected for processing (1)
  • ktor-utils/api/ktor-utils.api

📝 Walkthrough

Walkthrough

Switched Zstd encode/decode helpers to use a direct ByteBuffer pool and replaced manual frame-size probing with direct streaming compression/decompression using pooled direct ByteBuffers; added a default direct ByteBuffer pool and adjusted a test to exercise smaller direct buffers.

Changes

Cohort / File(s) Summary
Zstd encoder/decoder
ktor-shared/ktor-encoding-zstd/jvm/src/io/ktor/encoding/zstd/Zstd.jvm.kt
Replaced KtorDefaultPool with KtorDefaultDirectPool; removed frame-size detection and byte-array-based (de)compression; implemented streaming decompressDirectByteBufferStream / compress loops using pooled direct ByteBuffers; added/reused a second pooled output buffer and removed related helpers/imports.
Buffer pool API
ktor-utils/jvm/src/io/ktor/util/cio/ByteBufferPool.kt, ktor-utils/api/ktor-utils.api
Added exported default direct buffer pool KtorDefaultDirectPool (DirectByteBufferPool(...)) and its public accessor in the API surface.
Tests
ktor-shared/ktor-encoding-zstd/jvm/test/io/ktor/encoding/zstd/ZstdTest.kt
Changed test to use DirectByteBufferPool(capacity = 10, bufferSize = 5) to exercise decoding with smaller direct buffers.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested reviewers

  • bjhham
  • osipxd
🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main changes: improving zstd performance and fixing a specific issue (KTOR-9487), which aligns with the changeset's primary objectives.
Description check ✅ Passed The description follows the template with all required sections (Subsystem, Motivation, Solution) present and adequately filled. It explains the problem, references the issue, and describes the solution clearly.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@ktor-shared/ktor-encoding-zstd/jvm/src/io/ktor/encoding/zstd/Zstd.jvm.kt`:
- Around line 103-112: The fixed 4098-byte outputBuf can overflow for
incompressible input when calling ctx.compress(outputBuf, inputBuf) and the
return code is ignored; change the logic in the loop around
ctx.compress/outputBuf/inputBuf to allocate or ensure outputBuf.capacity() >=
Zstd.compressBound(inputBuf.remaining()) (or switch to the streaming compressor
API), call ctx.compress and check its return value for errors (handle negative
error codes by throwing/logging or reallocating a larger outputBuf), and then
proceed to flip and writeFully only after a successful compress.

In `@ktor-utils/jvm/src/io/ktor/util/cio/ByteBufferPool.kt`:
- Around line 20-25: The new public JVM symbol KtorDefaultDirectPool (and its
use of DirectByteBufferPool/DEFAULT_KTOR_POOL_SIZE/DEFAULT_BUFFER_SIZE) is
missing from the ABI signature files; run the Gradle task to update the legacy
ABI so the new symbol is recorded by executing ./gradlew
:ktor-utils:updateLegacyAbi (or the project’s ABI update task), then commit the
modified ktor-utils/api/ktor-utils.api so KtorDefaultDirectPool is included in
the published ABI signatures.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: a9ad9c2c-077d-4fb0-9a01-9ccac2de817b

📥 Commits

Reviewing files that changed from the base of the PR and between 9e2a691 and a09de41.

📒 Files selected for processing (3)
  • ktor-shared/ktor-encoding-zstd/jvm/src/io/ktor/encoding/zstd/Zstd.jvm.kt
  • ktor-shared/ktor-encoding-zstd/jvm/test/io/ktor/encoding/zstd/ZstdTest.kt
  • ktor-utils/jvm/src/io/ktor/util/cio/ByteBufferPool.kt

Comment thread ktor-utils/jvm/src/io/ktor/util/cio/ByteBufferPool.kt
Signed-off-by: solonovamax <solonovamax@12oclockpoint.com>
Signed-off-by: solonovamax <solonovamax@12oclockpoint.com>
@osipxd osipxd self-assigned this Apr 12, 2026
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