From 289130b8a7760b813c7bcfdd37f978a17f10c31a Mon Sep 17 00:00:00 2001 From: Amr Mahdi Date: Mon, 26 Oct 2020 04:46:48 +0000 Subject: [PATCH 1/3] Improve image pull performance from http 1.1 container registries Private registries that does not support http 2.0 such as Azure Container Registry streams back content in a max of 16KB chunks (max TLS record size). The small chunks introduce an overhead when copying the layers to the content store sine each chunk incurs the overhead of grpc message that has to be sent to the content store. This change reduces this overhead by buffering the chunks into 1MB chunks and only then writes a message to the content store. Below is a per comparsion between the 2 approaches using a couple of large images that are being pulled from the docker hub (http 2.0) and a private Azure CR (http 1.1) in seconds. image | Buffered copy | master ------- |---------------|---------- docker.io/pytorch/pytorch:latest | 55.63 | 58.33 docker.io/nvidia/cuda:latest | 72.05 | 75.98 containerdpulltest.azurecr.io/pytorch/pytorch:latest | 61.45 | 77.1 containerdpulltest.azurecr.io/nvidia/cuda:latest | 77.13 | 85.47 Signed-off-by: Amr Mahdi --- content/helpers.go | 29 ++++++++++++++++++++++++++--- 1 file changed, 26 insertions(+), 3 deletions(-) diff --git a/content/helpers.go b/content/helpers.go index c1c204618..46ca556e9 100644 --- a/content/helpers.go +++ b/content/helpers.go @@ -230,8 +230,31 @@ func seekReader(r io.Reader, offset, size int64) (io.Reader, error) { } func copyWithBuffer(dst io.Writer, src io.Reader) (written int64, err error) { - buf := bufPool.Get().(*[]byte) - written, err = io.CopyBuffer(dst, src, *buf) - bufPool.Put(buf) + bufRef := bufPool.Get().(*[]byte) + defer bufPool.Put(bufRef) + buf := *bufRef + for { + nr, er := io.ReadAtLeast(src, buf, len(buf)) + if nr > 0 { + nw, ew := dst.Write(buf[0:nr]) + if nw > 0 { + written += int64(nw) + } + if ew != nil { + err = ew + break + } + if nr != nw { + err = io.ErrShortWrite + break + } + } + if er != nil { + if er != io.EOF && er != io.ErrUnexpectedEOF { + err = er + } + break + } + } return } From f6834d4c0b9a0433d6723ea172ad333ddc44936b Mon Sep 17 00:00:00 2001 From: Amr Mahdi Date: Tue, 27 Oct 2020 01:25:36 +0000 Subject: [PATCH 2/3] replicate io.Copy optimizations Signed-off-by: Amr Mahdi --- content/helpers.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/content/helpers.go b/content/helpers.go index 46ca556e9..d15c98224 100644 --- a/content/helpers.go +++ b/content/helpers.go @@ -230,6 +230,15 @@ func seekReader(r io.Reader, offset, size int64) (io.Reader, error) { } func copyWithBuffer(dst io.Writer, src io.Reader) (written int64, err error) { + // If the reader has a WriteTo method, use it to do the copy. + // Avoids an allocation and a copy. + if wt, ok := src.(io.WriterTo); ok { + return wt.WriteTo(dst) + } + // Similarly, if the writer has a ReadFrom method, use it to do the copy. + if rt, ok := dst.(io.ReaderFrom); ok { + return rt.ReadFrom(src) + } bufRef := bufPool.Get().(*[]byte) defer bufPool.Put(bufRef) buf := *bufRef From b81917ee72a8e705127006084619b5c0ef76aa8e Mon Sep 17 00:00:00 2001 From: Amr Mahdi Date: Tue, 3 Nov 2020 04:23:52 +0000 Subject: [PATCH 3/3] Add comments clarifying copyWithBuffer implementation Signed-off-by: Amr Mahdi --- content/helpers.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/content/helpers.go b/content/helpers.go index d15c98224..4c4a35308 100644 --- a/content/helpers.go +++ b/content/helpers.go @@ -229,6 +229,10 @@ func seekReader(r io.Reader, offset, size int64) (io.Reader, error) { return r, nil } +// copyWithBuffer is very similar to io.CopyBuffer https://golang.org/pkg/io/#CopyBuffer +// but instead of using Read to read from the src, we use ReadAtLeast to make sure we have +// a full buffer before we do a write operation to dst to reduce overheads associated +// with the write operations of small buffers. func copyWithBuffer(dst io.Writer, src io.Reader) (written int64, err error) { // If the reader has a WriteTo method, use it to do the copy. // Avoids an allocation and a copy. @@ -259,6 +263,8 @@ func copyWithBuffer(dst io.Writer, src io.Reader) (written int64, err error) { } } if er != nil { + // If an EOF happens after reading fewer than the requested bytes, + // ReadAtLeast returns ErrUnexpectedEOF. if er != io.EOF && er != io.ErrUnexpectedEOF { err = er }