Skip to content

util/compression: use zstd.EncoderLevelFromZstd to map compression levels#6933

Open
thaJeztah wants to merge 1 commit into
moby:masterfrom
thaJeztah:zstd_mapping
Open

util/compression: use zstd.EncoderLevelFromZstd to map compression levels#6933
thaJeztah wants to merge 1 commit into
moby:masterfrom
thaJeztah:zstd_mapping

Conversation

@thaJeztah

@thaJeztah thaJeztah commented Jul 7, 2026

Copy link
Copy Markdown
Member

The toZstdEncoderLevel utility was introduced in cab33b1, and has a custom mapping of zstd compressions levels to their go (klauspost/compress) counterparts.

Swap our local implementation for the one in klauspost/compress so that new levels implemented in klauspost/compress will also be mapped if they arrive.

This does change some mappings;

Input level Before After
< 0 SpeedDefault SpeedFastest
6 SpeedDefault SpeedBetterCompression
9 SpeedBestCompression SpeedBetterCompression

While updating, also slightly refactor;

  • use an options slice to skip setting the option if no config was set, and to allow adding additional options.
  • inline zstdWriter where used, as it was only used in a single place.

Input level Before (toZstdEncoderLevel) After (zstd.EncoderLevelFromZstd) Change
< 0 SpeedDefault SpeedFastest ⚠️ Different
0–2 SpeedFastest SpeedFastest ✅ Same
3–5 SpeedDefault SpeedDefault ✅ Same
6 SpeedDefault SpeedBetterCompression ⚠️ Different
7–8 SpeedBetterCompression SpeedBetterCompression ✅ Same
9 SpeedBestCompression SpeedBetterCompression ⚠️ Different

= 10 | SpeedBestCompression | SpeedBestCompression | ✅ Same

@thaJeztah

Copy link
Copy Markdown
Member Author

Test failure output looks a bit wonky; for some reason it does print %, but no value?

buildkit/client/build_test.go

Lines 1175 to 1189 in 64bbec8

func (p *testPrompt) wait(msg string) string {
for {
newOutput := p.output.String()[p.pos:]
if strings.Contains(newOutput, msg) {
p.pos += len(newOutput)
return newOutput
}
select {
case <-p.ctx.Done():
p.t.Logf("Output at timeout: %s", p.output.String())
p.t.Fatalf("Timeout waiting for %q", msg)
case <-time.After(100 * time.Millisecond):
}
}
}

=== FAIL: client TestClientGatewayIntegration/TestClientGatewayContainerCancelExecTty/worker=containerd-1.7 (12.95s)
    build_test.go:1184: Output at timeout: % 
    build_test.go:1185: Timeout waiting for "% "
    build_test.go:1355: 
        	Error Trace:	/src/client/build_test.go:1355
        	            				/src/util/testutil/integration/run.go:105
        	            				/src/util/testutil/integration/run.go:253
        	Error:      	An error is expected but got nil.

@thaJeztah

Copy link
Copy Markdown
Member Author

From discussion on Slack to see if there was anything to match w.r.t. behavior, but looks like containerd at least does not set any options at all, so always uses the defaults; https://github.com/containerd/containerd/blob/d8f13bf4cce8b8a99ae61457cb574696fcf475e5/pkg/archive/compression/compression.go#L244-L256

// CompressStream compresses the dest with specified compression algorithm.
func CompressStream(dest io.Writer, compression Compression) (io.WriteCloser, error) {
switch compression {
case Uncompressed:
return &writeCloserWrapper{dest, nil}, nil
case Gzip:
return gzip.NewWriter(dest), nil
case Zstd:
return zstd.NewWriter(dest)
default:
return nil, fmt.Errorf("unsupported compression format %s", (&compression).Extension())
}
}

@thaJeztah thaJeztah marked this pull request as ready for review July 7, 2026 16:58
…vels

The toZstdEncoderLevel utility was introduced in cab33b1,
and has a custom mapping of zstd compressions levels to their go (klauspost/compress)
counterparts.

Swap our local implementation for the one in klauspost/compress so that new levels
implemented in klauspost/compress will also be mapped if they arrive.

This does change some mappings;

| Input level | Before                 | After                    |
|-------------|------------------------|--------------------------|
| `< 0`       | `SpeedDefault`         | `SpeedFastest`           |
| `6`         | `SpeedDefault`         | `SpeedBetterCompression` |
| `9`         | `SpeedBestCompression` | `SpeedBetterCompression` |

While updating, also slightly refactor;

- use an options slice to skip setting the option if no config was set, and
  to allow adding additional options.
- inline zstdWriter where used, as it was only used in a single place.

Signed-off-by: Sebastiaan van Stijn <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant