From 34378ec9b4579c9f647dd6724fcb04b3390bc3dd Mon Sep 17 00:00:00 2001 From: Kazuyoshi Kato Date: Thu, 5 Oct 2023 22:11:14 -0700 Subject: [PATCH 1/2] Use Intel ISA-L's igzip if available Intel ISA-L is Intel's open source (BSD) library that outperforms both gzip and pigz. This commit checks and uses igzip if available. Signed-off-by: Kazuyoshi Kato --- archive/compression/benchmark_test.go | 28 +++++++++++++------ archive/compression/compression.go | 36 ++++++++++++++----------- archive/compression/compression_test.go | 10 +++---- 3 files changed, 46 insertions(+), 28 deletions(-) diff --git a/archive/compression/benchmark_test.go b/archive/compression/benchmark_test.go index f8340802a..845f5f9e8 100644 --- a/archive/compression/benchmark_test.go +++ b/archive/compression/benchmark_test.go @@ -20,6 +20,7 @@ import ( "fmt" "io" "net/http" + "os/exec" "testing" "github.com/stretchr/testify/require" @@ -49,19 +50,30 @@ func BenchmarkDecompression(b *testing.B) { zstd := testCompress(b, data, Zstd) b.Run(fmt.Sprintf("size=%dMiB", sizeInMiB), func(b *testing.B) { - b.Run("gzip", func(b *testing.B) { - testDecompress(b, gz) - }) + original := gzipPath + defer func() { + gzipPath = original + }() + b.Run("zstd", func(b *testing.B) { testDecompress(b, zstd) }) - if unpigzPath != "" { - original := unpigzPath - unpigzPath = "" - b.Run("gzipPureGo", func(b *testing.B) { + + gzipPath = "" + b.Run("gzipPureGo", func(b *testing.B) { + testDecompress(b, gz) + }) + gzipPath, err = exec.LookPath("igzip") + if err == nil { + b.Run("igzip", func(b *testing.B) { + testDecompress(b, gz) + }) + } + gzipPath, err = exec.LookPath("unpigz") + if err == nil { + b.Run("unpigz", func(b *testing.B) { testDecompress(b, gz) }) - unpigzPath = original } }) } diff --git a/archive/compression/compression.go b/archive/compression/compression.go index fda32e0e4..b69efcd7e 100644 --- a/archive/compression/compression.go +++ b/archive/compression/compression.go @@ -47,11 +47,14 @@ const ( Zstd ) -const disablePigzEnv = "CONTAINERD_DISABLE_PIGZ" +const ( + disablePigzEnv = "CONTAINERD_DISABLE_PIGZ" + disableIgzipEnv = "CONTAINERD_DISABLE_IGZIP" +) var ( - initPigz sync.Once - unpigzPath string + initGzip sync.Once + gzipPath string ) var ( @@ -259,17 +262,20 @@ func (compression *Compression) Extension() string { } func gzipDecompress(ctx context.Context, buf io.Reader) (io.ReadCloser, error) { - initPigz.Do(func() { - if unpigzPath = detectPigz(); unpigzPath != "" { - log.L.Debug("using pigz for decompression") + initGzip.Do(func() { + if gzipPath = detectCommand("igzip", disableIgzipEnv); gzipPath != "" { + log.L.Debug("using igzip for decompression") + return + } + if gzipPath = detectCommand("unpigz", disablePigzEnv); gzipPath != "" { + log.L.Debug("using unpigz for decompression") } }) - if unpigzPath == "" { + if gzipPath == "" { return gzip.NewReader(buf) } - - return cmdStream(exec.CommandContext(ctx, unpigzPath, "-d", "-c"), buf) + return cmdStream(exec.CommandContext(ctx, gzipPath, "-d", "-c"), buf) } func cmdStream(cmd *exec.Cmd, in io.Reader) (io.ReadCloser, error) { @@ -296,22 +302,22 @@ func cmdStream(cmd *exec.Cmd, in io.Reader) (io.ReadCloser, error) { return reader, nil } -func detectPigz() string { - path, err := exec.LookPath("unpigz") +func detectCommand(path, disableEnvName string) string { + path, err := exec.LookPath(path) if err != nil { - log.L.WithError(err).Debug("unpigz not found, falling back to go gzip") + log.L.WithError(err).Debugf("%s not found", path) return "" } - // Check if pigz disabled via CONTAINERD_DISABLE_PIGZ env variable - value := os.Getenv(disablePigzEnv) + // Check if this command is disabled via the env variable + value := os.Getenv(disableEnvName) if value == "" { return path } disable, err := strconv.ParseBool(value) if err != nil { - log.L.WithError(err).Warnf("could not parse %s: %s", disablePigzEnv, value) + log.L.WithError(err).Warnf("could not parse %s: %s", disableEnvName, value) return path } diff --git a/archive/compression/compression_test.go b/archive/compression/compression_test.go index fe1a62e35..4f89487ca 100644 --- a/archive/compression/compression_test.go +++ b/archive/compression/compression_test.go @@ -102,9 +102,9 @@ func testCompressDecompress(t testing.TB, size int, compression Compression) Dec } func TestCompressDecompressGzip(t *testing.T) { - oldUnpigzPath := unpigzPath - unpigzPath = "" - defer func() { unpigzPath = oldUnpigzPath }() + oldUnpigzPath := gzipPath + gzipPath = "" + defer func() { gzipPath = oldUnpigzPath }() decompressor := testCompressDecompress(t, 1024*1024, Gzip) wrapper := decompressor.(*readCloserWrapper) @@ -148,7 +148,7 @@ func TestDetectPigz(t *testing.T) { t.Setenv("PATH", tempPath) - if pigzPath := detectPigz(); pigzPath == "" { + if pigzPath := detectCommand("unpigz", disablePigzEnv); pigzPath == "" { t.Fatal("failed to detect pigz path") } else if pigzPath != fullPath { t.Fatalf("wrong pigz found: %s != %s", pigzPath, fullPath) @@ -156,7 +156,7 @@ func TestDetectPigz(t *testing.T) { t.Setenv(disablePigzEnv, "1") - if pigzPath := detectPigz(); pigzPath != "" { + if pigzPath := detectCommand("unpigz", disablePigzEnv); pigzPath != "" { t.Fatalf("disable via %s doesn't work", disablePigzEnv) } } From 535916d1d0a586acad617ceb37f07be487a09847 Mon Sep 17 00:00:00 2001 From: kaz Date: Fri, 6 Oct 2023 09:20:08 -0700 Subject: [PATCH 2/2] Skip exec.LookPath if a specific gzip implementation is disabled Both pigz and igzip can be disabled via the environment variables. If disabled, calling exec.LookPath and logging "not found" message is, even in the debug level, doesn't make much sense. Signed-off-by: Kazuyoshi Kato --- archive/compression/compression.go | 29 +++++++++++++---------------- 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/archive/compression/compression.go b/archive/compression/compression.go index b69efcd7e..0caf710ae 100644 --- a/archive/compression/compression.go +++ b/archive/compression/compression.go @@ -303,27 +303,24 @@ func cmdStream(cmd *exec.Cmd, in io.Reader) (io.ReadCloser, error) { } func detectCommand(path, disableEnvName string) string { + // Check if this command is disabled via the env variable + value := os.Getenv(disableEnvName) + if value != "" { + disable, err := strconv.ParseBool(value) + if err != nil { + log.L.WithError(err).Warnf("could not parse %s: %s", disableEnvName, value) + } + + if disable { + return "" + } + } + path, err := exec.LookPath(path) if err != nil { log.L.WithError(err).Debugf("%s not found", path) return "" } - // Check if this command is disabled via the env variable - value := os.Getenv(disableEnvName) - if value == "" { - return path - } - - disable, err := strconv.ParseBool(value) - if err != nil { - log.L.WithError(err).Warnf("could not parse %s: %s", disableEnvName, value) - return path - } - - if disable { - return "" - } - return path }