Skip to content

Commit e9d77e0

Browse files
dfa1claude
andcommitted
docs: add architecture decision records (ADR 0001-0014)
Bootstrap docs/adr/ in MADR 3.0 format (ADR.md index + template.md) and record the project's foundational decisions retrospectively, so contributors can see why the project is shaped the way it is. Covers: FFM over JNI (0001), zig cc native build (0002), MemorySegment-first API (0003), per-classifier native jars (0004), no lib-path override (0005), native compile flags (0006), integration tests as ground truth (0007), size_t/sentinel mapping (0008), NativeObject idempotent close (0009), bounded native-context pool for virtual threads (0010, Proposed), JPMS module (0011, Accepted - not yet on main), benchmark methodology (0012), binding coverage scope (0013), single-threaded native build (0014). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 3dfd5b8 commit e9d77e0

16 files changed

Lines changed: 760 additions & 0 deletions

docs/adr/0001-ffm-over-jni.md

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
# ADR 0001: FFM bindings over JNI
2+
3+
- **Status:** Completed
4+
- **Date:** 2026-06-27
5+
- **Deciders:** project maintainer
6+
7+
## Context
8+
9+
Calling the zstd C library from Java needs a native bridge. The established
10+
option is JNI (what `zstd-jni` uses): hand-written or generated C glue compiled
11+
per platform. JDK 25 ships the stable Foreign Function & Memory API
12+
(`java.lang.foreign`), which calls C directly from Java with no C glue.
13+
14+
## Decision
15+
16+
Use the FFM API exclusively. No JNI, no hand-written C, no generated stubs.
17+
Native symbols bind to `MethodHandle`s in `Bindings`; the library targets JDK
18+
25+ where `java.lang.foreign` is stable.
19+
20+
## Consequences
21+
22+
### Positive
23+
- Zero C source to maintain, review, or compile for the *bindings* (only zstd
24+
itself is compiled — see ADR 0002).
25+
- Enables the zero-copy `MemorySegment` path (ADR 0003) — JNI must copy across
26+
the boundary; FFM can pass off-heap addresses directly.
27+
- Pure-Java artifact; native code is only the bundled `libzstd`.
28+
29+
### Negative
30+
- Hard floor at JDK 25. No JDK 17/21 support.
31+
- FFM downcalls are a restricted operation: callers must pass
32+
`--enable-native-access`.
33+
34+
### Risks to manage
35+
- FFM is young; signatures/restrictions may evolve. Centralised in `Bindings`
36+
and `NativeCall` so changes are localised.
37+
38+
## Alternatives considered
39+
40+
- **JNI (zstd-jni):** mature, but ships hand-written C and copies at the
41+
boundary, foreclosing zero-copy.
42+
- **JNA / JNR:** reflection-based FFI, slower per call, also pre-FFM.
43+
44+
## References
45+
46+
- [Bindings.java], [NativeCall.java]
47+
- [ADR 0003 — segment-first API](0003-segment-first-api.md)
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
# ADR 0002: Build the native library with `zig cc`
2+
3+
- **Status:** Completed
4+
- **Date:** 2026-06-27
5+
- **Deciders:** project maintainer
6+
7+
## Context
8+
9+
The project bundles `libzstd` for six classifiers (`{osx,linux,windows}` ×
10+
`{x86_64,aarch64}`). zstd is pure C with no build-system dependency. The build
11+
must produce all six from CI without per-platform runners or system toolchains.
12+
13+
## Decision
14+
15+
Compile the zstd library sources directly with `zig cc`
16+
(`scripts/build-zstd.sh`). Zig bundles clang + libc + headers for every target,
17+
so any host cross-compiles any classifier hermetically. No zstd Makefile, no
18+
CMake, no sysroot. The Maven `exec` plugin runs it in `generate-resources`;
19+
it is idempotent (skips if the library exists).
20+
21+
## Consequences
22+
23+
### Positive
24+
- One CI host (`ubuntu-latest`) builds all six classifiers — including the ones
25+
with no free CI runner (windows-aarch64, linux-aarch64).
26+
- Hermetic and reproducible: no dependence on the host's installed toolchain.
27+
- Compiles `.c` directly, bypassing zstd's own build entirely.
28+
29+
### Negative
30+
- Adds a Zig toolchain dependency to the build.
31+
- `zig cc` is a clang wrapper, not the zstd-blessed build path.
32+
33+
### Risks to manage
34+
- Zig is pre-1.0; `zig cc` flag behaviour can shift between versions. The Zig
35+
version is **pinned** (0.16.0) in CI; upgrades require a re-test.
36+
37+
## Alternatives considered
38+
39+
- **GitHub matrix runners:** no free windows-aarch64 / fragmented, slow.
40+
- **CMake/Make + cross sysroots:** per-target toolchain setup, the misery this
41+
decision exists to avoid.
42+
43+
## References
44+
45+
- [scripts/build-zstd.sh](../../scripts/build-zstd.sh)
46+
- [ADR 0006 — native compile flags](0006-native-compile-flags.md)
47+
- [ADR 0014 — single-threaded native build](0014-single-threaded-native-build.md)

docs/adr/0003-segment-first-api.md

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
# ADR 0003: MemorySegment-first API with thin `byte[]` overloads
2+
3+
- **Status:** Completed
4+
- **Date:** 2026-06-27
5+
- **Deciders:** project maintainer
6+
7+
## Context
8+
9+
Callers fall into two camps: heap callers holding `byte[]`, and zero-copy
10+
callers whose bytes are already off-heap (an mmap slice in, an arena buffer
11+
out). A `byte[]`-only API forces the second camp to copy on and off heap on
12+
every call — the dominant per-op cost for native compression.
13+
14+
## Decision
15+
16+
Make the `MemorySegment` path primary and add **thin** `byte[]` overloads for
17+
heap callers. The segment path never allocates a `byte[]` for decode output.
18+
`NativeCall.requireNative` guards zero-copy entry points so a stray heap
19+
segment fails fast with a clear message.
20+
21+
## Consequences
22+
23+
### Positive
24+
- Zero-copy callers get an allocation-free path (~0 B/op in benchmarks) —
25+
the project's differentiator over `byte[]`-only bindings.
26+
- Heap callers keep an ergonomic `byte[]` API.
27+
28+
### Negative
29+
- Two overload families to document and test.
30+
- Segment correctness rules (native vs heap, capacity) are on the caller.
31+
32+
### Risks to manage
33+
- Misuse: passing a heap segment to a zero-copy method. Mitigated by
34+
`requireNative`.
35+
36+
## Alternatives considered
37+
38+
- **`byte[]`-only (aircompressor-style):** simplest, but forecloses zero-copy —
39+
the reason this library exists.
40+
- **`ByteBuffer`-first:** weaker lifetime/ownership model than `Arena` +
41+
`MemorySegment`.
42+
43+
## References
44+
45+
- [docs/zero-copy.md](../zero-copy.md), [docs/benchmarks.md](../benchmarks.md)
46+
- [ADR 0001 — FFM over JNI](0001-ffm-over-jni.md)
Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
# ADR 0004: Per-classifier native JARs
2+
3+
- **Status:** Completed
4+
- **Date:** 2026-06-27
5+
- **Deciders:** project maintainer
6+
7+
## Context
8+
9+
The bundled `libzstd` must reach the JVM at runtime. A single fat jar carrying
10+
all six platform libraries bloats every download ~6x. The library also has to
11+
load correctly both on the classpath and as a named JPMS module (ADR 0011).
12+
13+
## Decision
14+
15+
Ship one `zstd-native-<classifier>` artifact per platform, each packaging only
16+
its own `libzstd.{so,dylib,dll}` under `native/<classifier>/`. At runtime
17+
`NativeLibrary` computes the classifier from `os.name`/`os.arch`, loads the
18+
resource via the class loader, extracts it to an owner-only temp dir, and
19+
`dlopen`s it once at class-init. Callers depend on the library jar plus the one
20+
native jar for their platform (or via the BOM).
21+
22+
## Consequences
23+
24+
### Positive
25+
- A consumer downloads ~one platform's library, not six.
26+
- The classifier directory name contains a dash, so it is not a valid package
27+
and the resource escapes module encapsulation — readable across modules and
28+
on the classpath alike.
29+
30+
### Negative
31+
- Consumers must select the right native artifact (eased by the BOM).
32+
- Six native modules to build and publish.
33+
34+
### Risks to manage
35+
- New CPU arch silently mis-resolving: `classifier()` throws a clear
36+
`UnsatisfiedLinkError` for unsupported arches rather than guessing.
37+
38+
## Alternatives considered
39+
40+
- **Fat jar (all platforms):** simplest for consumers, 6x bloat for everyone.
41+
- **System-installed libzstd:** version drift, no hermetic guarantee.
42+
43+
## References
44+
45+
- [NativeLibrary.java], [bom/](../../bom)
46+
- [ADR 0005 — no lib-path override](0005-no-lib-path-override.md)
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
# ADR 0005: No `-Dzstd.lib.path` override
2+
3+
- **Status:** Completed
4+
- **Date:** 2026-06-27
5+
- **Deciders:** project maintainer
6+
7+
## Context
8+
9+
A common convenience in native-binding libraries is a system property pointing
10+
at an alternative shared library (`-Dzstd.lib.path=/path/to/libzstd`). It is
11+
handy for testing a self-built library — but loading a caller-supplied native
12+
library is arbitrary native-code execution in the JVM process.
13+
14+
## Decision
15+
16+
Provide **no** path override. `NativeLibrary` loads only the bundled artifact
17+
from the classpath, extracts it into a private owner-only (`0700`) temp
18+
directory created atomically, and `dlopen`s that. To run a self-built
19+
`libzstd`, the user repackages the native resource jar — documented in
20+
`docs/how-to.md`.
21+
22+
## Consequences
23+
24+
### Positive
25+
- No code path turns an attacker-controlled string into `dlopen` of an
26+
arbitrary library.
27+
- Owner-only temp dir prevents a local user swapping the library between
28+
extraction and load (TOCTOU).
29+
30+
### Negative
31+
- Less convenient for ad-hoc library swaps; requires repackaging.
32+
33+
### Risks to manage
34+
- Temp-dir permissions are POSIX-only; on Windows the per-user temp root
35+
provides the equivalent isolation.
36+
37+
## Alternatives considered
38+
39+
- **`-Dzstd.lib.path` override:** convenient, but an RCE-shaped foot-gun.
40+
- **Allow override only when a flag is set:** still ships the dangerous path.
41+
42+
## Implementation
43+
44+
- `9e505c6` / `8d0ea6a` (#8) — bundled-only native loader; removed any path
45+
override, load solely from the classpath artifact.
46+
- `b2ed28b` — set owner-only (`0700`) temp dir permissions atomically (Sonar
47+
S5443).
48+
- `c9929b9` (#20) — follow-up Sonar SECURITY-category fixes.
49+
50+
## References
51+
52+
- [NativeLibrary.java], [docs/how-to.md](../how-to.md)
53+
- [ADR 0004 — per-classifier native jars](0004-per-classifier-native-jars.md)
Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
# ADR 0006: Native compile flags
2+
3+
- **Status:** Completed
4+
- **Date:** 2026-06-27
5+
- **Deciders:** project maintainer
6+
7+
## Context
8+
9+
Compiling zstd directly (ADR 0002) means owning the compile/link flags that the
10+
zstd build system would otherwise set. The flags must produce identical codegen
11+
across six targets and keep the exported symbol surface minimal.
12+
13+
## Decision
14+
15+
Compile with:
16+
17+
- `-O3 -DNDEBUG` — release codegen.
18+
- `-DZSTD_DISABLE_ASM=1` — drop the x86-only `.S` files, giving identical C
19+
codegen on every target.
20+
- `-DXXH_NAMESPACE=ZSTD_` — matches zstd's own build, avoids xxhash symbol
21+
clashes.
22+
- `-fPIC`.
23+
24+
Visibility differs by platform:
25+
26+
- **ELF / Mach-O:** `-fvisibility=hidden`; zstd's `ZSTDLIB_VISIBLE` keeps only
27+
the public API exported.
28+
- **Windows / MinGW:** *no* hidden visibility (it suppresses PE exports);
29+
instead `-Wl,--export-all-symbols` lets lld populate the PE export table.
30+
31+
Symbol/debug tables are stripped at link (`-s` on ELF); the lld-emitted `.pdb`
32+
and import `.lib` on Windows are deleted after link so they are not bundled.
33+
34+
## Consequences
35+
36+
### Positive
37+
- Deterministic, minimal-surface libraries; stripped ELF is ~6x smaller.
38+
39+
### Negative
40+
- `DISABLE_ASM` may leave marginal x86 performance on the table vs the
41+
assembly paths (not measured to matter at level 3).
42+
43+
### Risks to manage
44+
- The Windows-vs-ELF visibility split is non-obvious; a wrong flag silently
45+
empties the Windows export table. Documented inline in the build script.
46+
47+
## Alternatives considered
48+
49+
- **Keep `.S` asm per target:** divergent codegen, x86-only, build complexity.
50+
- **Hidden visibility on Windows too:** breaks PE exports — rejected.
51+
52+
## References
53+
54+
- [scripts/build-zstd.sh](../../scripts/build-zstd.sh)
55+
- [ADR 0002 — zig cc native build](0002-zig-cc-native-build.md)
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
# ADR 0007: Integration tests as ground truth
2+
3+
- **Status:** Completed
4+
- **Date:** 2026-06-27
5+
- **Deciders:** project maintainer
6+
7+
## Context
8+
9+
There is no machine-checkable formal spec for the zstd frame format that a
10+
binding can assert against. Unit tests prove internal consistency but not that
11+
output is *correct zstd* that other tools accept.
12+
13+
## Decision
14+
15+
Treat integration tests as ground truth, on two pillars:
16+
17+
1. **Interop with `zstd-jni`** (luben — the zstd C library via JNI): every
18+
encoding round-trip is cross-checked — what we compress, JNI decompresses,
19+
and vice versa.
20+
2. **The vendored golden corpus** under `third_party/zstd/tests/`
21+
(`golden-compression`, `golden-decompression`,
22+
`golden-decompression-errors`, `golden-dictionaries`) — the canonical,
23+
version-matched fixtures the C project itself ships.
24+
25+
Unit tests stay fast and pure (no I/O, network, sleep). Every encoding
26+
round-trip and file-format boundary gets an integration test.
27+
28+
## Consequences
29+
30+
### Positive
31+
- Correctness defined against an independent implementation and the upstream
32+
corpus, not our own assumptions.
33+
- Error-case fixtures pin our handling of malformed frames.
34+
35+
### Negative
36+
- Integration tests need the git submodule and the `zstd-jni` dependency.
37+
- Slower than the unit suite; kept in a separate module.
38+
39+
### Risks to manage
40+
- Corpus drifts with the submodule version; pinned via the submodule SHA.
41+
42+
## Alternatives considered
43+
44+
- **Unit tests only:** cannot prove cross-tool correctness.
45+
- **Round-trip against ourselves only:** a self-consistent bug stays invisible.
46+
47+
## References
48+
49+
- [integration-tests/](../../integration-tests)
50+
- [third_party/zstd/tests/](../../third_party/zstd/tests)
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
# ADR 0008: `size_t` maps to `JAVA_LONG`, guard sentinels
2+
3+
- **Status:** Completed
4+
- **Date:** 2026-06-27
5+
- **Deciders:** project maintainer
6+
7+
## Context
8+
9+
zstd's C API uses `size_t` and `unsigned long long` pervasively, and encodes
10+
errors as `size_t` values near `SIZE_MAX` (checked via `ZSTD_isError`). Java has
11+
no unsigned types. The binding must map these consistently and not mistake an
12+
error sentinel for a huge valid size.
13+
14+
## Decision
15+
16+
Model `size_t` and `unsigned long long` as `ValueLayout.JAVA_LONG` (LP64). All
17+
native handles live in `Bindings`. Every public method that returns a
18+
size-typed value routes through `NativeCall.checkReturnValue`, which calls
19+
`ZSTD_isError` and throws `ZstdException` (carrying the decoded `ZstdErrorCode`)
20+
before the value is interpreted as a length. Negative-interpreted sentinels are
21+
guarded at the public boundary.
22+
23+
## Consequences
24+
25+
### Positive
26+
- One place (`NativeCall`) owns the error convention; binding classes stay
27+
uniform.
28+
- Sentinels become typed Java exceptions, not silent giant lengths.
29+
30+
### Negative
31+
- Values above `Long.MAX_VALUE` are not representable — acceptable: such sizes
32+
exceed any realistic JVM allocation and zstd's own limits.
33+
34+
### Risks to manage
35+
- A new binding that skips `checkReturnValue` would leak raw sentinels — the
36+
convention is documented and shared.
37+
38+
## Alternatives considered
39+
40+
- **`JAVA_INT` for sizes:** overflows on payloads >2 GB.
41+
- **Per-call ad-hoc error checks:** duplicated, easy to forget.
42+
43+
## References
44+
45+
- [Bindings.java], [NativeCall.java], [ZstdException.java], [ZstdErrorCode.java]

0 commit comments

Comments
 (0)