Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request optimizes the async writer for small payloads by deferring the creation of async goroutines until they're actually needed. It also removes the external gammazero/workerpool dependency and replaces it with a custom, lightweight worker pool implementation.
Changes:
- Introduced a custom worker pool implementation in
internal/pkg/wpoolto replace the external dependency - Modified async writer to defer async task creation until a second block is needed, using synchronous compression for single-block payloads
- Added comprehensive tests for user-provided dictionaries with both linked and independent block modes
- Added comparative benchmarks between plz4 and lz4 libraries
- Optimized bakeoff benchmark execution order to reduce cache penalties
Reviewed changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| internal/pkg/wpool/wpool.go | New custom worker pool implementation with dynamic scaling, idle worker cleanup, and panic recovery |
| internal/pkg/wpool/wpool_test.go | Comprehensive test suite for worker pool functionality including concurrency, GC, panic handling |
| internal/pkg/async/writer.go | Short-circuit optimization: defer async tasks until second block, use sync path for small payloads |
| internal/pkg/async/reader.go | Limit decompression tasks based on content size when known |
| internal/pkg/compress/compress.go | Added documentation for Compressor interface |
| internal/test/wr_test.go | Added dictionary application tests, updated worker pool API usage |
| internal/test/wr_bench_test.go | Added comparative benchmarks between plz4 and lz4 |
| internal/test/rd_test.go | Added test case with max parallel setting |
| internal/test/block_test.go | Enhanced compression benchmarks with multiple input sizes and levels |
| cmd/plz4/internal/ops/bakeoff.go | Reversed benchmark execution order to reduce cache penalties, removed worker pool usage |
| go.mod, go.sum | Removed gammazero/workerpool dependency |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Run tests in reverse order so slowest samples run first. This prevents skewing of results on the fatest samples due to caching effects. - Remove the worker pool; spawning goroutines is faster.
When reading or writing data asynchronously, if the operation can be completed immediately (i.e., without blocking), we now short-circuit the process. This optimization reduces latency and improves throughput for scenarios where data is readily available.
scunningham
force-pushed
the
short_circuit_async
branch
from
c948a9d to
23b9a5e
Compare
aleksmaus
approved these changes
Jan 31, 2026
| @@ -171,7 +172,7 @@ func benchmarkWrite(b *testing.B, sz int, opts ...Option) { | |||
| b.ReportAllocs() | |||
| b.ResetTimer() | |||
| for i := 0; i < b.N; i++ { | |||
There was a problem hiding this comment.
nit: could "modernize"
was used in the different file as
for b.Loop() {
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.