Skip to content

Kill the naked int/long size: ZstdByteSize value type#98

Open
dfa1 wants to merge 1 commit into
mainfrom
feature/zstd-byte-size
Open

Kill the naked int/long size: ZstdByteSize value type#98
dfa1 wants to merge 1 commit into
mainfrom
feature/zstd-byte-size

Conversation

@dfa1

@dfa1 dfa1 commented Jul 22, 2026

Copy link
Copy Markdown
Owner

Summary

  • Adds ZstdByteSize, a record wrapping a byte size/count as a validated non-negative long, with toIntExact() for callers that must narrow to an int because the result ends up in a byte[] (throws ArithmeticException past Integer.MAX_VALUE, matching Math.toIntExact).
  • Replaces every naked int/long size parameter with it: Zstd.decompress, Zstd.compressBound, Zstd.estimateCompressDictSize/estimateDecompressDictSize, ZstdDictionary.train/trainCover/trainFastCover/finalizeFrom, ZstdOutputStream.withPledgedSize.
  • Centralizes interpretation of zstd's raw frame-content-size results as ZstdByteSize factories rather than scattered sentinel checks:
    • ZstdByteSize.fromFrameContentSize(long) turns the ZSTD_getFrameContentSize/ZSTD_findDecompressedSize/ZSTD_decompressBound sentinels — and any other negative reading (an unsigned value ≥ 2^63) — into a ZstdException instead of a bogus negative size or the wrong exception type. Used by Zstd.decompress(byte[]), Zstd.decompressedSize(MemorySegment), and ZstdFrame.decompressedSize/decompressedBound, which now return ZstdByteSize instead of unwrapping immediately back to long.
    • ZstdByteSize.fromFrameHeaderContentSize(long) backs ZstdFrameHeader.contentSize(), now Optional<ZstdByteSize> instead of OptionalLong.
    • The CONTENTSIZE_UNKNOWN/CONTENTSIZE_ERROR sentinel constants move from Zstd to become a private implementation detail of ZstdByteSize.
  • ZstdFrame's other size-returning methods (compressedSize, decompressionMargin, headerSize) are unaffected — they never construct a ZstdByteSize, no sentinel semantics involved.
  • Breaking change (pre-1.0, no compatibility shims). CHANGELOG updated under [Unreleased].

Closes #96.

Design notes

  • One long-backed type, not two split by width — narrowing happens at the four array-bounded call sites via toIntExact(); native/streaming totals pass long straight through. See discussion on Consider a size value type for byte-size parameters #96.
  • A real regression was caught and fixed during review: a frame declaring a content size with the sign bit set (unsigned ≥ 2^63) was falling through both named sentinel checks and throwing the wrong exception type (IllegalArgumentException instead of the documented ZstdException). Covered by a new regression test (ZstdTest.UntrustedInput.rejectsAFrameDeclaringAContentSizeWithTheSignBitSet).

Test plan

  • ./mvnw -pl zstd,integration-tests,benchmark -am clean verify — build success, all unit + integration tests pass
  • ./mvnw -pl zstd validate — checkstyle clean
  • ./mvnw -pl zstd javadoc:javadoc — zero output (failOnError/failOnWarnings)
  • Added ZstdByteSizeTest covering construction, negative rejection, toIntExact() in-range/overflow
  • Manually reproduced and verified the sign-bit-set regression fix against a hand-crafted frame
  • Multiple independent review passes focused on native-call boundary correctness, exception-type consistency, and Javadoc/call-site accuracy — no outstanding issues

🤖 Generated with Claude Code

/// [#toIntExact()] rather than silently truncating.
///
/// @param value the byte size or count, non-negative
public record ZstdByteSize(long value) {

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

maybe it should be called ZstdSize?

Comment thread README.md Outdated

List<byte[]> samples = ...; // representative records
ZstdDictionary dict = ZstdDictionary.train(samples, 8 * 1024);
ZstdDictionary dict = ZstdDictionary.train(samples, new ZstdByteSize(8 * 1024));

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

maybe we should also expose a better way to build 8K or 1M constants? this could be useful for tests internally but also for end users to define the buffer size for copies, etc

Introduces ZstdByteSize, a record wrapping a byte size/count as a validated
non-negative long, so that holding one guarantees validity instead of leaving
callers to wonder whether a raw long from the library still needs checking.
Provides toIntExact() for callers that must narrow to an int because the
result ends up in a byte[] (throws ArithmeticException past
Integer.MAX_VALUE, matching Math.toIntExact), and ofKiB(long)/ofMiB(long)
factories for common buffer sizes instead of spelling out arithmetic like
`new ZstdByteSize(16 * 1024)`.

Replaces every naked int/long size parameter, and every long return value
that represents a byte count, across the public API: Zstd.compressBound,
Zstd.estimateCompressContextSize/estimateDecompressContextSize/
estimateCompressDictSize/estimateDecompressDictSize,
Zstd.decompressedSize(MemorySegment), ZstdDictionary.train/trainCover/
trainFastCover/finalizeFrom, ZstdOutputStream.withPledgedSize,
ZstdFrame.decompressedSize/decompressedBound, and sizeOf() on
ZstdCompressContext/ZstdDecompressContext/ZstdCompressStream/
ZstdDecompressStream/ZstdCompressDictionary/ZstdDecompressDictionary.

Centralizes interpretation of zstd's raw frame-content-size results as
ZstdByteSize factories rather than scattered sentinel checks:
- ZstdByteSize.fromFrameContentSize(long) turns the
  ZSTD_getFrameContentSize/ZSTD_findDecompressedSize/ZSTD_decompressBound
  sentinels (and any other negative reading, i.e. an unsigned value >= 2^63)
  into a ZstdException instead of a bogus negative size or the wrong
  exception type.
- ZstdByteSize.fromFrameHeaderContentSize(long) backs
  ZstdFrameHeader.contentSize(), now Optional<ZstdByteSize> instead of
  OptionalLong.
The CONTENTSIZE_UNKNOWN/CONTENTSIZE_ERROR sentinel constants move from Zstd
to become a private implementation detail of ZstdByteSize.

Breaking change; the library is pre-1.0 so no compatibility shims are added.

Closes #96.
@dfa1
dfa1 force-pushed the feature/zstd-byte-size branch from 3de81d6 to b4773df Compare July 22, 2026 06:14
@dfa1

dfa1 commented Jul 22, 2026

Copy link
Copy Markdown
Owner Author

Re: ZstdSize naming — kept ZstdByteSize. It matches the house convention of descriptive multi-word names (ZstdCompressionLevel, ZstdDictionaryId, ZstdFrameHeader) — "Size" alone felt more ambiguous (could read as a count, a dimension) where "ByteSize" says exactly what's inside.

Re: KB/MB constants — added ZstdByteSize.ofKiB(long) / ofMiB(long) (using Math.multiplyExact so an overflow fails fast instead of wrapping to a bogus size), and swapped the naked new ZstdByteSize(16 * 1024)-style literals across README/docs/tests/smoke for it. Pushed as b4773df.

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.

Consider a size value type for byte-size parameters

1 participant