Repository navigation
Conversation
The Go reader's per-DATA XZ cost comes from the 8 MiB dictionary the Go writer declares, not from reader configuration. Records measurements and the writer dictionary-size options for a user decision.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the
|
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 33 |
| Duplication | 0 |
AI Reviewer: first review requested successfully. AI can make mistakes. Always validate suggestions.
TIP This summary will be updated as you push new changes.
There was a problem hiding this comment.
Pull Request Overview
✅ No blocking issues noticed
No blocking findings were identified. The analysis documents the allocation behavior and recommendation, but several acceptance areas remain unverified, including reproducible benchmarks, compatibility checks, and validation of the proposed writer configuration. No production-code or test changes are included, so these should be addressed before implementation or as part of the follow-up validation.
About this PR
- The reported results still need a reproducible measurement harness, including separately retained-memory data and compatibility/round-trip validation before the documented recommendation is implemented.
Test suggestions
- Reproduce XZ decode timing and allocation across the documented dictionary sizes.
- Verify existing Go-written files remain readable without changing their declared dictionary size.
- Validate the proposed 256 KiB writer configuration, including cross-language and large-payload round trips.
Prompt proposal for missing tests
Consider implementing these tests if applicable:
1. Reproduce XZ decode timing and allocation across the documented dictionary sizes.
2. Verify existing Go-written files remain readable without changing their declared dictionary size.
3. Validate the proposed 256 KiB writer configuration, including cross-language and large-payload round trips.
TIP Improve review quality by adding custom instructions
TIP How was this review? Give us feedback
There was a problem hiding this comment.
All reported issues were addressed across 3 files
Architecture diagram
sequenceDiagram
participant GoWriter as Go Writer
participant RustWriter as Rust Writer
participant Systemd as systemd journald
participant XZLib as ulikunitz/xz v0.5.16
participant GoReader as Go Reader
participant LZMA2 as LZMA2 Filter Reader
participant Alloc as Memory Allocator
participant SOW as SOW Tracker
Note over GoWriter,RustWriter: NEW: Documented dictionary size discrepancy
GoWriter->>XZLib: xzCompress() - DictCap unset (default 8 MiB)
XZLib->>XZLib: Uses library default 8 MiB dictionary
XZLib-->>GoWriter: XZ stream with 8 MiB declared dict
RustWriter->>XZLib: XzOptions::with_preset(0) - 256 KiB dict
XZLib-->>RustWriter: XZ stream with 256 KiB declared dict
Systemd->>XZLib: compress.c:180 - 1 MiB dict
XZLib-->>Systemd: XZ stream with 1 MiB declared dict
Note over GoReader,Alloc: NEW: Root cause - allocation follows stream declaration
GoReader->>XZLib: Open XZ stream
XZLib->>LZMA2: Initialize filter reader
LZMA2->>LZMA2: Read stream header (declared dict size)
alt Declared 8 MiB (Go writer)
LZMA2->>Alloc: Allocate + zero 8.42 MB
Alloc-->>LZMA2: Dictionary buffer
Note over LZMA2: Decode time: ~1.5 ms
else Declared 1 MiB (systemd)
LZMA2->>Alloc: Allocate + zero 1.08 MB
Alloc-->>LZMA2: Dictionary buffer
Note over LZMA2: Decode time: ~0.45 ms
else Declared 256 KiB (Rust writer)
LZMA2->>Alloc: Allocate + zero 0.30 MB
Alloc-->>LZMA2: Dictionary buffer
Note over LZMA2: Decode time: ~0.17 ms
end
LZMA2-->>XZLib: Ready with fixed dictionary
XZLib-->>GoReader: Per-DATA decompression
Note over GoReader: NEW: Reader DictCap has no effect (max(reader cap, stream size))
alt Reader attempts to set DictCap to minimum
GoReader->>LZMA2: Set DictCap to 64 KiB
LZMA2->>LZMA2: Still uses stream declared size
end
Note over GoReader: No decoder reuse available (no reset API)
Note over SOW: NEW: Decision D1 recorded
SOW->>SOW: Option A: 256 KiB (recommended, matches Rust)
SOW->>SOW: Option B: 1 MiB (matches systemd)
SOW->>SOW: Option C: keep 8 MiB
Note over SOW: Existing files keep declared size under all options
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
…emory; frame the gate as D1 - Compression loss of a 256 KiB dictionary is under 0.1% up to 1 MiB synthetic payloads and 0.75% at 4 MiB; allocation follows the declared dictionary. - Retained memory returns to baseline after each decode; peak is the dictionary. - Gate, plans and open decisions now describe the writer dictionary decision. - Root index restores the not-activated / not-authorized guard.
There was a problem hiding this comment.
All reported issues were addressed across 2 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
# Conflicts: # .agents/sow/SOW-status.md
There was a problem hiding this comment.
All reported issues were addressed across 1 file (changes from recent commits).
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
Every XZ reader allocates the declared dictionary per DATA decode, and the library default declared 8 MiB. 256 KiB matches the Rust writer and cuts Go and Rust read cost of new Go-written XZ files 5-10x; decoded content is unchanged and older files still read.
There was a problem hiding this comment.
All reported issues were addressed across 6 files (changes from recent commits).
Requires human review: Auto-approval blocked because this review re-detected 1 unresolved issue already reported by Cubic.
View guided diff | Turn on auto-fix | Re-trigger cubic
Stock verify seal tests in both SDKs now skip when journalctl was built without forward-secure sealing, so negative tests cannot pass for the wrong reason. A new test proves 1 MiB and 8 MiB XZ streams still decode. The SOW and ledger report per-reader CPU figures and qualify the unmeasured stock-reader claim.
Stacked on #19. Completes SOW-0160 (
.agents/sow/done/SOW-0160-20261015-go-xz-reader-dictionary-allocation.md).Decision
Why
ulikunitz/xzv0.5.16 uses the larger of the reader's cap and the stream's declared size, and it has no reset API for reusing a decoder.Changes
go/journal/writer_compression.go:xzDictCap = 256 << 10is now set inxzCompress.go/journal/writer_compression_xz_test.go:product-scope.md) anddocs/Options-Reference.mdrecord the declared dictionary.Results
Test file: 5,000 Go-written XZ rows, exported with
journalctl --file F -o export:Validation
go test ./...andcargo test --workspacepass.Summary by cubic
The Go XZ writer now declares a 256 KiB LZMA2 dictionary instead of the library's 8 MiB default, matching the Rust writer. Reads of new Go-written XZ files cost the Go reader ~6x less time and ~28x less allocation per small decode, and the Rust journalctl runs ~10x faster; decoded content is unchanged and older 8 MiB files still read.
xzCompresssets the dictionary; no other writer path or dependency changes.journalctlwas built without sealing support, so negative seal tests cannot pass for the wrong reason.journalctlwould benefit: liblzma decodes 8 MiB and 256 KiB files at the same cost.Written for commit ee9c461. Summary will update on new commits.