Skip to content

Add native Zstd input and output streams#315

Open
dain wants to merge 2 commits intoairlift:masterfrom
dain:native-zstd-streaming
Open

Add native Zstd input and output streams#315
dain wants to merge 2 commits intoairlift:masterfrom
dain:native-zstd-streaming

Conversation

@dain
Copy link
Member

@dain dain commented Jan 25, 2026

No description provided.

@dain dain requested review from electrum and martint January 25, 2026 00:04
@dain dain force-pushed the native-zstd-streaming branch 2 times, most recently from 3ed8972 to c287744 Compare January 25, 2026 03:15
this.useNative = useNative;
}

private void createDecompressingStreamIfNecessary()
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not pre-create in constructor?
(Might be worth code comment)

Copy link
Member Author

Choose a reason for hiding this comment

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

There is a good reason to delay construction here.

  • ZstdNativeInputStream allocates native stream state in its constructor (createDecompressStream + arena setup), so lazy init avoids that cost when a codec stream is created but never actually read.
  • It also avoids re-allocating immediately on resetState(); current code resets to null and recreates on next read

I added comments

this(true); // Default to native when available
}

public ZstdHadoopStreams(boolean useNative)
Copy link
Contributor

Choose a reason for hiding this comment

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

useNative → canUseNative

Copy link
Member Author

Choose a reason for hiding this comment

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

This is what the other hadoop streams use, like Lz4HadoopStreams.java (line 30)

import static java.util.Objects.requireNonNull;
import static sun.misc.Unsafe.ARRAY_BYTE_BASE_OFFSET;

public final class ZstdJavaInputStream
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible to structure git changes in a way that this doesn't appear as all new code?
(Perhaps rename as separate commit?)

Copy link
Member Author

Choose a reason for hiding this comment

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

Good idea. Done!

@dain dain force-pushed the native-zstd-streaming branch from c287744 to 20b0a4e Compare March 5, 2026 22:38
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