Improved memory consumption during extraction of Zip files

Author: cmaglie

extract.go

@@ -27,7 +27,6 @@
 package extract
 
 import (
-	"bytes"
 	"context"
 	"io"
 	"os"
@@ -70,11 +69,6 @@ func Gz(ctx context.Context, body io.Reader, location string, rename Renamer) er
 	return extractor.Gz(ctx, body, location, rename)
 }
 
-type file struct {
-	Path string
-	Mode os.FileMode
-	Data bytes.Buffer
-}
 type link struct {
 	Name string
 	Path string

extract_test.go

@@ -382,6 +382,33 @@ func TestTarGzMemoryConsumption(t *testing.T) {
 	require.True(t, heapUsed < 5000000, "heap consumption should be less than 5M but is %d", heapUsed)
 }
 
+func TestZipMemoryConsumption(t *testing.T) {
+	archive := paths.New("testdata/big.zip")
+	err := download(t, "http://downloads.arduino.cc/tools/gcc-arm-none-eabi-7-2017-q4-major-win32-arduino1.zip", archive)
+	require.NoError(t, err)
+
+	tmpDir, err := paths.MkTempDir("", "")
+	require.NoError(t, err)
+	defer tmpDir.RemoveAll()
+
+	f, err := archive.Open()
+	require.NoError(t, err)
+
+	var m, m2 runtime.MemStats
+	runtime.GC()
+	runtime.ReadMemStats(&m)
+
+	err = extract.Zip(context.Background(), f, tmpDir.String(), nil)
+	require.NoError(t, err)
+
+	runtime.ReadMemStats(&m2)
+	heapUsed := m2.HeapInuse - m.HeapInuse
+	fmt.Println("Heap memory used during the test:", heapUsed)
+	// the .zip file require random access, so the full io.Reader content must be cached, since
+	// the test file is 130MB, that's the reason for the 180+ Mb of memory consumed.
+	require.True(t, heapUsed < 200000000, "heap consumption should be less than 200M but is %d", heapUsed)
+}
+
 func download(t require.TestingT, url string, file *paths.Path) error {
 	if file.Exist() {
 		return nil

extractor.go

@@ -196,7 +196,6 @@ func (e *Extractor) Zip(ctx context.Context, body io.Reader, location string, re
 		return errors.Annotatef(err, "Read the zip file")
 	}
 
-	files := []file{}
 	links := []link{}
 
 	// We make the first pass creating the directory structure, or we could end up
@@ -239,35 +238,26 @@ func (e *Extractor) Zip(ctx context.Context, body io.Reader, location string, re
 			}
 		// We only check for symlinks because hard links aren't possible
 		case info.Mode()&os.ModeSymlink != 0:
-			f, err := header.Open()
-			if err != nil {
+			if f, err := header.Open(); err != nil {
 				return errors.Annotatef(err, "Open link %s", path)
-			}
-			name, err := ioutil.ReadAll(f)
-			if err != nil {
+			} else if name, err := ioutil.ReadAll(f); err != nil {
 				return errors.Annotatef(err, "Read address of link %s", path)
+			} else {
+				links = append(links, link{Path: path, Name: string(name)})
+				f.Close()
 			}
-			links = append(links, link{Path: path, Name: string(name)})
 		default:
-			f, err := header.Open()
-			if err != nil {
+			if f, err := header.Open(); err != nil {
 				return errors.Annotatef(err, "Open file %s", path)
+			} else if err := e.copy(ctx, path, info.Mode(), f); err != nil {
+				return errors.Annotatef(err, "Create file %s", path)
+			} else {
+				f.Close()
 			}
-			var data bytes.Buffer
-			if _, err := copyCancel(ctx, &data, f); err != nil {
-				return errors.Annotatef(err, "Read contents of file %s", path)
-			}
-			files = append(files, file{Path: path, Mode: info.Mode(), Data: data})
-		}
-	}
-
-	// Now we make another pass creating the files and links
-	for i := range files {
-		if err := e.copy(ctx, files[i].Path, files[i].Mode, &files[i].Data); err != nil {
-			return errors.Annotatef(err, "Create file %s", files[i].Path)
 		}
 	}
 
+	// Now we make another pass creating the links
 	for i := range links {
 		select {
 		case <-ctx.Done():