Refactor to optimize storage driver ApplyDiff()

To avoid an expensive call to archive.ChangesDirs() which walks two directory
trees and compares every entry, archive.ApplyLayer() has been extended to
also return the size of the layer changes.

Docker-DCO-1.1-Signed-off-by: Josh Hawn <josh.hawn@docker.com> (github: jlhawn)

daemon/graphdriver/aufs/aufs.go

@@ -312,7 +312,7 @@ func (a *Driver) applyDiff(id string, diff archive.ArchiveReader) error {
 // DiffSize calculates the changes between the specified id
 // and its parent and returns the size in bytes of the changes
 // relative to its base filesystem directory.
-func (a *Driver) DiffSize(id, parent string) (bytes int64, err error) {
+func (a *Driver) DiffSize(id, parent string) (size int64, err error) {
 	// AUFS doesn't need the parent layer to calculate the diff size.
 	return utils.TreeSize(path.Join(a.rootPath(), "diff", id))
 }
@@ -320,7 +320,7 @@ func (a *Driver) DiffSize(id, parent string) (bytes int64, err error) {
 // ApplyDiff extracts the changeset from the given diff into the
 // layer with the specified id and parent, returning the size of the
 // new layer in bytes.
-func (a *Driver) ApplyDiff(id, parent string, diff archive.ArchiveReader) (bytes int64, err error) {
+func (a *Driver) ApplyDiff(id, parent string, diff archive.ArchiveReader) (size int64, err error) {
 	// AUFS doesn't need the parent id to apply the diff.
 	if err = a.applyDiff(id, diff); err != nil {
 		return

daemon/graphdriver/driver.go

@@ -63,11 +63,11 @@ type Driver interface {
 	// ApplyDiff extracts the changeset from the given diff into the
 	// layer with the specified id and parent, returning the size of the
 	// new layer in bytes.
-	ApplyDiff(id, parent string, diff archive.ArchiveReader) (bytes int64, err error)
+	ApplyDiff(id, parent string, diff archive.ArchiveReader) (size int64, err error)
 	// DiffSize calculates the changes between the specified id
 	// and its parent and returns the size in bytes of the changes
 	// relative to its base filesystem directory.
-	DiffSize(id, parent string) (bytes int64, err error)
+	DiffSize(id, parent string) (size int64, err error)
 }
 
 var (

daemon/graphdriver/fsdiff.go

@@ -3,14 +3,12 @@
 package graphdriver
 
 import (
-	"fmt"
 	"time"
 
 	log "github.com/Sirupsen/logrus"
 	"github.com/docker/docker/pkg/archive"
 	"github.com/docker/docker/pkg/chrootarchive"
 	"github.com/docker/docker/pkg/ioutils"
-	"github.com/docker/docker/utils"
 )
 
 // naiveDiffDriver takes a ProtoDriver and adds the
@@ -27,8 +25,8 @@ type naiveDiffDriver struct {
 // it may or may not support on its own:
 //     Diff(id, parent string) (archive.Archive, error)
 //     Changes(id, parent string) ([]archive.Change, error)
-//     ApplyDiff(id, parent string, diff archive.ArchiveReader) (bytes int64, err error)
-//     DiffSize(id, parent string) (bytes int64, err error)
+//     ApplyDiff(id, parent string, diff archive.ArchiveReader) (size int64, err error)
+//     DiffSize(id, parent string) (size int64, err error)
 func NaiveDiffDriver(driver ProtoDriver) Driver {
 	return &naiveDiffDriver{ProtoDriver: driver}
 }
@@ -111,7 +109,7 @@ func (gdw *naiveDiffDriver) Changes(id, parent string) ([]archive.Change, error)
 // ApplyDiff extracts the changeset from the given diff into the
 // layer with the specified id and parent, returning the size of the
 // new layer in bytes.
-func (gdw *naiveDiffDriver) ApplyDiff(id, parent string, diff archive.ArchiveReader) (bytes int64, err error) {
+func (gdw *naiveDiffDriver) ApplyDiff(id, parent string, diff archive.ArchiveReader) (size int64, err error) {
 	driver := gdw.ProtoDriver
 
 	// Mount the root filesystem so we can apply the diff/layer.
@@ -123,34 +121,18 @@ func (gdw *naiveDiffDriver) ApplyDiff(id, parent string, diff archive.ArchiveRea
 
 	start := time.Now().UTC()
 	log.Debugf("Start untar layer")
-	if err = chrootarchive.ApplyLayer(layerFs, diff); err != nil {
+	if size, err = chrootarchive.ApplyLayer(layerFs, diff); err != nil {
 		return
 	}
 	log.Debugf("Untar time: %vs", time.Now().UTC().Sub(start).Seconds())
 
-	if parent == "" {
-		return utils.TreeSize(layerFs)
-	}
-
-	parentFs, err := driver.Get(parent, "")
-	if err != nil {
-		err = fmt.Errorf("Driver %s failed to get image parent %s: %s", driver, parent, err)
-		return
-	}
-	defer driver.Put(parent)
-
-	changes, err := archive.ChangesDirs(layerFs, parentFs)
-	if err != nil {
-		return
-	}
-
-	return archive.ChangesSize(layerFs, changes), nil
+	return
 }
 
 // DiffSize calculates the changes between the specified layer
 // and its parent and returns the size in bytes of the changes
 // relative to its base filesystem directory.
-func (gdw *naiveDiffDriver) DiffSize(id, parent string) (bytes int64, err error) {
+func (gdw *naiveDiffDriver) DiffSize(id, parent string) (size int64, err error) {
 	driver := gdw.ProtoDriver
 
 	changes, err := gdw.Changes(id, parent)

daemon/graphdriver/overlay/overlay.go

@@ -28,7 +28,7 @@ var (
 
 type ApplyDiffProtoDriver interface {
 	graphdriver.ProtoDriver
-	ApplyDiff(id, parent string, diff archive.ArchiveReader) (bytes int64, err error)
+	ApplyDiff(id, parent string, diff archive.ArchiveReader) (size int64, err error)
 }
 
 type naiveDiffDriverWithApply struct {
@@ -309,7 +309,7 @@ func (d *Driver) Put(id string) {
 	delete(d.active, id)
 }
 
-func (d *Driver) ApplyDiff(id string, parent string, diff archive.ArchiveReader) (bytes int64, err error) {
+func (d *Driver) ApplyDiff(id string, parent string, diff archive.ArchiveReader) (size int64, err error) {
 	dir := d.dir(id)
 
 	if parent == "" {
@@ -347,7 +347,7 @@ func (d *Driver) ApplyDiff(id string, parent string, diff archive.ArchiveReader)
 		return 0, err
 	}
 
-	if err := chrootarchive.ApplyLayer(tmpRootDir, diff); err != nil {
+	if size, err = chrootarchive.ApplyLayer(tmpRootDir, diff); err != nil {
 		return 0, err
 	}
 
@@ -356,12 +356,7 @@ func (d *Driver) ApplyDiff(id string, parent string, diff archive.ArchiveReader)
 		return 0, err
 	}
 
-	changes, err := archive.ChangesDirs(rootDir, parentRootDir)
-	if err != nil {
-		return 0, err
-	}
-
-	return archive.ChangesSize(rootDir, changes), nil
+	return
 }
 
 func (d *Driver) Exists(id string) bool {

pkg/archive/changes_test.go

@@ -286,7 +286,7 @@ func TestApplyLayer(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	if err := ApplyLayer(src, layerCopy); err != nil {
+	if _, err := ApplyLayer(src, layerCopy); err != nil {
 		t.Fatal(err)
 	}
 

pkg/archive/diff.go

@@ -15,7 +15,7 @@ import (
 	"github.com/docker/docker/pkg/system"
 )
 
-func UnpackLayer(dest string, layer ArchiveReader) error {
+func UnpackLayer(dest string, layer ArchiveReader) (size int64, err error) {
 	tr := tar.NewReader(layer)
 	trBuf := pools.BufioReader32KPool.Get(tr)
 	defer pools.BufioReader32KPool.Put(trBuf)
@@ -33,9 +33,11 @@ func UnpackLayer(dest string, layer ArchiveReader) error {
 			break
 		}
 		if err != nil {
-			return err
+			return 0, err
 		}
 
+		size += hdr.Size
+
 		// Normalize name, for safety and for a simple is-root check
 		hdr.Name = filepath.Clean(hdr.Name)
 
@@ -48,7 +50,7 @@ func UnpackLayer(dest string, layer ArchiveReader) error {
 			if _, err := os.Lstat(parentPath); err != nil && os.IsNotExist(err) {
 				err = os.MkdirAll(parentPath, 0600)
 				if err != nil {
-					return err
+					return 0, err
 				}
 			}
 		}
@@ -63,12 +65,12 @@ func UnpackLayer(dest string, layer ArchiveReader) error {
 				aufsHardlinks[basename] = hdr
 				if aufsTempdir == "" {
 					if aufsTempdir, err = ioutil.TempDir("", "dockerplnk"); err != nil {
-						return err
+						return 0, err
 					}
 					defer os.RemoveAll(aufsTempdir)
 				}
 				if err := createTarFile(filepath.Join(aufsTempdir, basename), dest, hdr, tr, true); err != nil {
-					return err
+					return 0, err
 				}
 			}
 			continue
@@ -77,10 +79,10 @@ func UnpackLayer(dest string, layer ArchiveReader) error {
 		path := filepath.Join(dest, hdr.Name)
 		rel, err := filepath.Rel(dest, path)
 		if err != nil {
-			return err
+			return 0, err
 		}
 		if strings.HasPrefix(rel, "..") {
-			return breakoutError(fmt.Errorf("%q is outside of %q", hdr.Name, dest))
+			return 0, breakoutError(fmt.Errorf("%q is outside of %q", hdr.Name, dest))
 		}
 		base := filepath.Base(path)
 
@@ -88,7 +90,7 @@ func UnpackLayer(dest string, layer ArchiveReader) error {
 			originalBase := base[len(".wh."):]
 			originalPath := filepath.Join(filepath.Dir(path), originalBase)
 			if err := os.RemoveAll(originalPath); err != nil {
-				return err
+				return 0, err
 			}
 		} else {
 			// If path exits we almost always just want to remove and replace it.
@@ -98,7 +100,7 @@ func UnpackLayer(dest string, layer ArchiveReader) error {
 			if fi, err := os.Lstat(path); err == nil {
 				if !(fi.IsDir() && hdr.Typeflag == tar.TypeDir) {
 					if err := os.RemoveAll(path); err != nil {
-						return err
+						return 0, err
 					}
 				}
 			}
@@ -113,18 +115,18 @@ func UnpackLayer(dest string, layer ArchiveReader) error {
 				linkBasename := filepath.Base(hdr.Linkname)
 				srcHdr = aufsHardlinks[linkBasename]
 				if srcHdr == nil {
-					return fmt.Errorf("Invalid aufs hardlink")
+					return 0, fmt.Errorf("Invalid aufs hardlink")
 				}
 				tmpFile, err := os.Open(filepath.Join(aufsTempdir, linkBasename))
 				if err != nil {
-					return err
+					return 0, err
 				}
 				defer tmpFile.Close()
 				srcData = tmpFile
 			}
 
 			if err := createTarFile(path, dest, srcHdr, srcData, true); err != nil {
-				return err
+				return 0, err
 			}
 
 			// Directory mtimes must be handled at the end to avoid further
@@ -139,27 +141,29 @@ func UnpackLayer(dest string, layer ArchiveReader) error {
 		path := filepath.Join(dest, hdr.Name)
 		ts := []syscall.Timespec{timeToTimespec(hdr.AccessTime), timeToTimespec(hdr.ModTime)}
 		if err := syscall.UtimesNano(path, ts); err != nil {
-			return err
+			return 0, err
 		}
 	}
-	return nil
+
+	return size, nil
 }
 
 // ApplyLayer parses a diff in the standard layer format from `layer`, and
-// applies it to the directory `dest`.
-func ApplyLayer(dest string, layer ArchiveReader) error {
+// applies it to the directory `dest`. Returns the size in bytes of the
+// contents of the layer.
+func ApplyLayer(dest string, layer ArchiveReader) (int64, error) {
 	dest = filepath.Clean(dest)
 
 	// We need to be able to set any perms
 	oldmask, err := system.Umask(0)
 	if err != nil {
-		return err
+		return 0, err
 	}
 	defer system.Umask(oldmask) // ignore err, ErrNotSupportedPlatform
 
 	layer, err = DecompressStream(layer)
 	if err != nil {
-		return err
+		return 0, err
 	}
 	return UnpackLayer(dest, layer)
 }

pkg/archive/utils_test.go

@@ -17,7 +17,8 @@ var testUntarFns = map[string]func(string, io.Reader) error{
 		return Untar(r, dest, nil)
 	},
 	"applylayer": func(dest string, r io.Reader) error {
-		return ApplyLayer(dest, ArchiveReader(r))
+		_, err := ApplyLayer(dest, ArchiveReader(r))
+		return err
 	},
 }
 

pkg/chrootarchive/archive_test.go

@@ -95,7 +95,7 @@ func TestChrootApplyEmptyArchiveFromSlowReader(t *testing.T) {
 		t.Fatal(err)
 	}
 	stream := &slowEmptyTarReader{size: 10240, chunkSize: 1024}
-	if err := ApplyLayer(dest, stream); err != nil {
+	if _, err := ApplyLayer(dest, stream); err != nil {
 		t.Fatal(err)
 	}
 }

pkg/chrootarchive/diff.go

@@ -1,6 +1,8 @@
 package chrootarchive
 
 import (
+	"bytes"
+	"encoding/json"
 	"flag"
 	"fmt"
 	"io"
@@ -14,6 +16,10 @@ import (
 	"github.com/docker/docker/pkg/reexec"
 )
 
+type applyLayerResponse struct {
+	LayerSize int64 `json:"layerSize"`
+}
+
 func applyLayer() {
 	runtime.LockOSThread()
 	flag.Parse()
@@ -21,6 +27,7 @@ func applyLayer() {
 	if err := chroot(flag.Arg(0)); err != nil {
 		fatal(err)
 	}
+
 	// We need to be able to set any perms
 	oldmask := syscall.Umask(0)
 	defer syscall.Umask(oldmask)
@@ -28,33 +35,53 @@ func applyLayer() {
 	if err != nil {
 		fatal(err)
 	}
+
 	os.Setenv("TMPDIR", tmpDir)
-	err = archive.UnpackLayer("/", os.Stdin)
+	size, err := archive.UnpackLayer("/", os.Stdin)
 	os.RemoveAll(tmpDir)
 	if err != nil {
 		fatal(err)
 	}
-	os.RemoveAll(tmpDir)
+
+	encoder := json.NewEncoder(os.Stdout)
+	if err := encoder.Encode(applyLayerResponse{size}); err != nil {
+		fatal(fmt.Errorf("unable to encode layerSize JSON: %s", err))
+	}
+
+	flush(os.Stdout)
 	flush(os.Stdin)
 	os.Exit(0)
 }
 
-func ApplyLayer(dest string, layer archive.ArchiveReader) error {
+func ApplyLayer(dest string, layer archive.ArchiveReader) (size int64, err error) {
 	dest = filepath.Clean(dest)
 	decompressed, err := archive.DecompressStream(layer)
 	if err != nil {
-		return err
+		return 0, err
 	}
+
 	defer func() {
 		if c, ok := decompressed.(io.Closer); ok {
 			c.Close()
 		}
 	}()
+
 	cmd := reexec.Command("docker-applyLayer", dest)
 	cmd.Stdin = decompressed
-	out, err := cmd.CombinedOutput()
-	if err != nil {
-		return fmt.Errorf("ApplyLayer %s %s", err, out)
+
+	outBuf, errBuf := new(bytes.Buffer), new(bytes.Buffer)
+	cmd.Stdout, cmd.Stderr = outBuf, errBuf
+
+	if err = cmd.Run(); err != nil {
+		return 0, fmt.Errorf("ApplyLayer %s stdout: %s stderr: %s", err, outBuf, errBuf)
+	}
+
+	// Stdout should be a valid JSON struct representing an applyLayerResponse.
+	response := applyLayerResponse{}
+	decoder := json.NewDecoder(outBuf)
+	if err = decoder.Decode(&response); err != nil {
+		return 0, fmt.Errorf("unable to decode ApplyLayer JSON response: %s", err)
 	}
-	return nil
+
+	return response.LayerSize, nil
 }