flate: Faster load+store by klauspost · Pull Request #1104 · klauspost/compress

klauspost · 2025-09-21T12:18:44Z

~10% faster huffman encoding.

Summary by CodeRabbit

Performance
- Optimized low-level read/write operations to reduce overhead and improve compression throughput.
- Removed unnecessary dependencies for leaner, faster execution.
Refactor
- Streamlined internal byte-handling logic for clearer, more efficient data writes.
Chores
- Internal maintenance to align implementations across safe/unsafe code paths.
Compatibility
- No user-facing behavior changes; existing workflows continue to work without modification.

coderabbitai · 2025-09-21T12:18:51Z

📝 Walkthrough

Walkthrough

Internal little-endian helper Store64 was made generic with an explicit offset parameter. Call sites in flate were updated to pass offsets. deflate.go switched 32-bit loads to le.Load32, removing encoding/binary usage. Unsafe-enabled/disabled implementations both updated to write at an indexed position.

Changes

Cohort / File(s)	Summary
flate: load/store callsites `flate/deflate.go`, `flate/huffman_bit_writer.go`	Replace `encoding/binary` 32-bit reads with `le.Load32`. Update all `le.Store64` calls to new signature with explicit offset parameter. Minor comment wording tweak.
internal/le: generic Store64 (unsafe-disabled) `internal/le/unsafe_disabled.go`	Change `Store64(b []byte, v uint64)` to `Store64[I Indexer](b []byte, i I, v uint64)`; write using `binary.LittleEndian.PutUint64(b[i:])`.
internal/le: generic Store64 (unsafe-enabled) `internal/le/unsafe_enabled.go`	Same signature change as above; write via `unsafe.Add(unsafe.Pointer(unsafe.SliceData(b)), i)`. Remove stale commented conversions.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant HW as HuffmanBitWriter
  participant Buf as Output Buffer ([]byte)
  participant LE as internal/le.Store64

  rect rgb(238,245,255)
  note over HW: Writing compressed bits
  HW->>LE: Store64(Buf, offset, bits)
  note right of LE: Compute address at Buf[offset:]
  LE-->>Buf: Write 8 bytes (little-endian)
  end

  note over HW,LE: Same flow under unsafe-enabled/disabled variants

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

flate: Cleanup & reduce casts #1050 — Touches internal/le and flate sites for LE helpers; similar migration of load/store utilities.
flate: Simplify matchlen (remove asm) #1045 — Introduces additional le.* usage (e.g., Load64 in matchlen) aligning with this PR’s move to internal LE helpers.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The title "flate: Faster load+store" is concise and accurately summarizes the primary change — performance-focused modifications to load and store paths in the flate package that enable faster Huffman encoding. It is scoped with the "flate:" prefix, avoids noise, and clearly communicates the intent to a teammate scanning history. The title aligns with the changes described in the diff (replacement of encoding/binary usage with optimized le.Load/Store and related signature updates).
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

📝 Generate Docstrings

🧪 Generate unit tests

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch flate-direct-writes

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.}

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (3)

internal/le/unsafe_disabled.go (1)
39-42: Docstring mismatch: now stores at b[i:], not b

The comment still says “Store64 will store v at b.” but the implementation writes at an offset (b[i:]). Update the comment to avoid confusion.

Apply:
-// Store64 will store v at b.
+// Store64 will store v at b[i:].
 func Store64[I Indexer](b []byte, i I, v uint64) {
Optional: consider adding a brief precondition note (len(b) >= int(i)+8) to mirror how call sites rely on headroom.
internal/le/unsafe_enabled.go (1)
49-52: Store64 now writes at an explicit offset — good API for hot paths

This matches the new call sites and avoids extra slicing/indexing. Given this uses unsafe.Add, it’s worth documenting the precondition that len(b) >= int(i)+8 and that i must be non-negative.

Add a brief precondition comment:
-// Store64 will store v at b[i:].
+// Store64 will store v at b[i:].
+// Precondition: len(b) >= int(i)+8 and i >= 0.
 func Store64[I Indexer](b []byte, i I, v uint64) {
flate/huffman_bit_writer.go (1)

437-439: Approve — no 2‑arg le.Store64 usages found; tiny follow-ups

Verified: all le.Store64 calls are three-argument (found at flate/huffman_bit_writer.go:438, 854, 881, 903, 928, 949, 1102, 1131).

Add a short comment by the bytes field declaration documenting the intentional +8 headroom (overwrite-8 / consume-6 invariant).

Optional: for consistency, change the call using n at flate/huffman_bit_writer.go:438 to nbytes.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6e2f5d5 and 5763b10.

📒 Files selected for processing (4)

flate/deflate.go (3 hunks)
flate/huffman_bit_writer.go (8 hunks)
internal/le/unsafe_disabled.go (1 hunks)
internal/le/unsafe_enabled.go (1 hunks)

🧰 Additional context used

🧬 Code graph analysis (4)

flate/deflate.go (2)

internal/le/unsafe_disabled.go (1)

Load32 (20-22)

internal/le/unsafe_enabled.go (1)

Load32 (26-30)

internal/le/unsafe_disabled.go (2)

internal/le/unsafe_enabled.go (1)

Store64 (50-52)

internal/le/le.go (1)

Indexer (3-5)

flate/huffman_bit_writer.go (2)

internal/le/unsafe_disabled.go (1)

Store64 (40-42)

internal/le/unsafe_enabled.go (1)

Store64 (50-52)

internal/le/unsafe_enabled.go (2)

internal/le/unsafe_disabled.go (1)

Store64 (40-42)

internal/le/le.go (1)

Indexer (3-5)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (17)

GitHub Check: Analyze (go)
GitHub Check: fuzz-other (nounsafe)
GitHub Check: build (1.25.x, ubuntu-latest)
GitHub Check: build (1.24.x, windows-latest)
GitHub Check: build (1.25.x, macos-latest)
GitHub Check: fuzz-other ("noasm,nounsafe")
GitHub Check: fuzz-zstd (nounsafe)
GitHub Check: build (1.23.x, windows-latest)
GitHub Check: fuzz-zstd ("noasm,nounsafe")
GitHub Check: fuzz-s2 (nounsafe)
GitHub Check: build (1.24.x, macos-latest)
GitHub Check: build (1.25.x, windows-latest)
GitHub Check: build (1.24.x, ubuntu-latest)
GitHub Check: build (1.23.x, macos-latest)
GitHub Check: fuzz-s2 ("noasm,nounsafe")
GitHub Check: build-special
GitHub Check: build (1.23.x, ubuntu-latest)

🔇 Additional comments (2)

flate/deflate.go (2)

14-15: Import shift to internal/le looks good

Dropping encoding/binary here and centralizing loads via internal/le is appropriate and aligns with the PR intent.

366-366: Swap to le.Load32 retains semantics and improves inlining

le.Load32(b, 0) is equivalent to binary.LittleEndian.Uint32(b) and should be faster with unsafe-enabled builds; caller precondition len(b) >= 4 still applies. Confirm there are no remaining binary.LittleEndian.Uint32 call sites in flate/ — the provided ripgrep check returned no matches (empty output); re-run or verify manually.

phuslu · 2025-09-23T13:04:07Z

It also could speedup gzip compression, Right? So any plan to release a tag for this performance improvement? Thanks!

klauspost · 2025-09-23T13:20:52Z

@phuslu Yes, but much less so, since entropy coding is only a (minor) part of it.

This is difference on "huffman" (last column is MiB/s)

enwik8	gzkp	-2	100000000	63193946	141	676.18
enwik8	gzkp	-2	100000000	63193946	124	768.79

This is the fastest compression...

enwik8	gzkp	1	100000000	41837916	456	208.88
enwik8	gzkp	1	100000000	41837916	449	212.16

I am reviewing code for golang/go#75532 - I will probably release once I've submitted that. For now the stdlib will not include "unsafe" code.

phuslu · 2025-09-23T13:25:51Z

Yes, that's true. We also are a modified k8s branch, it already integrated klauspost/compress for speedup gzip/zstd.

flate: Faster load+st0re

5763b10

klauspost changed the title ~~flate: Faster load+st0re~~ flate: Faster load+store Sep 21, 2025

coderabbitai bot reviewed Sep 21, 2025

View reviewed changes

klauspost merged commit 3c0d308 into master Sep 21, 2025
22 checks passed

klauspost deleted the flate-direct-writes branch September 21, 2025 12:44

m90 mentioned this pull request Oct 21, 2025

Bump github.com/klauspost/compress from 1.18.0 to 1.18.1 offen/docker-volume-backup#656

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

flate: Faster load+store#1104

flate: Faster load+store#1104
klauspost merged 1 commit intomasterfrom
flate-direct-writes

klauspost commented Sep 21, 2025 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Sep 21, 2025 •

edited

Loading

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Possibly related PRs

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

phuslu commented Sep 23, 2025

Uh oh!

klauspost commented Sep 23, 2025

Uh oh!

phuslu commented Sep 23, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

klauspost commented Sep 21, 2025 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Sep 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Possibly related PRs

Pre-merge checks and finishing touches

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

phuslu commented Sep 23, 2025

Uh oh!

klauspost commented Sep 23, 2025

Uh oh!

phuslu commented Sep 23, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

klauspost commented Sep 21, 2025 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Sep 21, 2025 •

edited

Loading