Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimization for the performance of addModules(), addExplodedModules(), and addFlattenModules() #1770

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 36 additions & 20 deletions internal/builder/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -582,21 +582,13 @@ func (b *Builder) addExplodedModules(kind string, logger logging.Logger, tmpDir
}

// create tar file
layerTar, err := buildpack.ToLayerTar(modTmpDir, module)
diffID, layerTar, err := buildpack.ToLayerTar(modTmpDir, module)
if err != nil {
lids[i] <- modInfo{err: err}
}

// generate diff id
diffID, err := dist.LayerDiffID(layerTar)
info := module.Descriptor().Info()
if err != nil {
lids[i] <- modInfo{err: errors.Wrapf(err,
"getting content hashes for %s %s",
kind,
style.Symbol(info.FullName()),
)}
}
lids[i] <- modInfo{
info: info,
layerTar: layerTar,
Expand Down Expand Up @@ -671,6 +663,7 @@ func (b *Builder) addExplodedModules(kind string, logger logging.Logger, tmpDir
func (b *Builder) addFlattenedModules(kind string, logger logging.Logger, tmpDir string, image imgutil.Image, flattenModules [][]buildpack.BuildModule, layers dist.ModuleLayers) ([]buildpack.BuildModule, error) {
collectionToAdd := map[string]toAdd{}
var (
diffID v1.Hash
buildModuleExcluded []buildpack.BuildModule
finalTarPath string
err error
Expand All @@ -679,21 +672,44 @@ func (b *Builder) addFlattenedModules(kind string, logger logging.Logger, tmpDir
buildModuleWriter := buildpack.NewBuildModuleWriter(logger, b.layerWriterFactory)
excludedModules := buildpack.Set(b.flattenExcludeBuildpacks)

type modInfo struct {
finalTarPath string
diffID v1.Hash
err error
}

lids := make([]chan modInfo, len(flattenModules))
for i := range lids {
lids[i] = make(chan modInfo, 1)
}

for i, additionalModules := range flattenModules {
modFlattenTmpDir := filepath.Join(tmpDir, fmt.Sprintf("%s-%s-flatten", kind, strconv.Itoa(i)))
if err := os.MkdirAll(modFlattenTmpDir, os.ModePerm); err != nil {
return nil, errors.Wrap(err, "creating flatten temp dir")
}
go func(i int, additionalModules []buildpack.BuildModule) {
// create directory
modFlattenTmpDir := filepath.Join(tmpDir, fmt.Sprintf("%s-%s-flatten", kind, strconv.Itoa(i)))
if err := os.MkdirAll(modFlattenTmpDir, os.ModePerm); err != nil {
lids[i] <- modInfo{err: errors.Wrap(err, "creating flatten temp dir")}
}

finalTarPath, buildModuleExcluded, err = buildModuleWriter.NToLayerTar(modFlattenTmpDir, fmt.Sprintf("%s-flatten-%s", kind, strconv.Itoa(i)), additionalModules, excludedModules)
if err != nil {
return nil, errors.Wrapf(err, "writing layer %s", finalTarPath)
}
diffID, finalTarPath, buildModuleExcluded, err = buildModuleWriter.NToLayerTar(modFlattenTmpDir, fmt.Sprintf("%s-flatten-%s", kind, strconv.Itoa(i)), additionalModules, excludedModules)
if err != nil {
lids[i] <- modInfo{err: errors.Wrapf(err, "writing layer %s", finalTarPath)}
}

diffID, err := dist.LayerDiffID(finalTarPath)
if err != nil {
return nil, errors.Wrapf(err, "calculating diff layer %s", finalTarPath)
// generate diff id
lids[i] <- modInfo{
finalTarPath: finalTarPath,
diffID: diffID,
}
}(i, additionalModules)
}

for i, additionalModules := range flattenModules {
mi := <-lids[i]
if mi.err != nil {
return nil, mi.err
}
diffID, finalTarPath := mi.diffID, mi.finalTarPath

for _, module := range additionalModules {
collectionToAdd[module.Descriptor().Info().FullName()] = toAdd{
Expand Down
14 changes: 1 addition & 13 deletions internal/fakes/fake_package.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@ import (
"os"
"path/filepath"

"github.com/google/go-containerregistry/pkg/v1/tarball"

"github.com/buildpacks/pack/pkg/buildpack"
"github.com/buildpacks/pack/pkg/dist"
)
Expand All @@ -28,17 +26,7 @@ type fakePackage struct {

func NewPackage(tmpDir string, name string, buildpacks []buildpack.BuildModule) (Package, error) {
processBuildpack := func(bp buildpack.BuildModule) (tarFile string, diffID string, err error) {
tarFile, err = buildpack.ToLayerTar(tmpDir, bp)
if err != nil {
return "", "", err
}

layer, err := tarball.LayerFromFile(tarFile)
if err != nil {
return "", "", err
}

hash, err := layer.DiffID()
hash, tarFile, err := buildpack.ToLayerTar(tmpDir, bp)
if err != nil {
return "", "", err
}
Expand Down
27 changes: 4 additions & 23 deletions pkg/buildpack/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,7 @@ func (b *PackageBuilder) finalizeImage(image WorkableImage, tmpDir string) error
var (
finalTarPath string
err error
diffID v1.Hash
)
for i, additionalModules := range b.FlattenedModules() {
modFlattenTmpDir := filepath.Join(tmpDir, fmt.Sprintf("buildpack-%s-flatten", strconv.Itoa(i)))
Expand All @@ -195,16 +196,11 @@ func (b *PackageBuilder) finalizeImage(image WorkableImage, tmpDir string) error
// include the buildpack itself
additionalModules = append(additionalModules, b.buildpack)
}
finalTarPath, individualBuildModules, err = buildModuleWriter.NToLayerTar(modFlattenTmpDir, fmt.Sprintf("buildpack-flatten-%s", strconv.Itoa(i)), additionalModules, excludedModules)
diffID, finalTarPath, individualBuildModules, err = buildModuleWriter.NToLayerTar(modFlattenTmpDir, fmt.Sprintf("buildpack-flatten-%s", strconv.Itoa(i)), additionalModules, excludedModules)
if err != nil {
return errors.Wrapf(err, "adding layer %s", finalTarPath)
}

diffID, err := dist.LayerDiffID(finalTarPath)
if err != nil {
return errors.Wrapf(err, "calculating diffID for layer %s", finalTarPath)
}

for _, module := range additionalModules {
collectionToAdd[module.Descriptor().Info().FullName()] = toAdd{
tarPath: finalTarPath,
Expand All @@ -221,18 +217,11 @@ func (b *PackageBuilder) finalizeImage(image WorkableImage, tmpDir string) error

// Let's create the tarball for each individual module
for _, bp := range append(b.dependencies.ExplodedModules(), individualBuildModules...) {
bpLayerTar, err := ToLayerTar(tmpDir, bp)
diffID, bpLayerTar, err := ToLayerTar(tmpDir, bp)
if err != nil {
return err
}

diffID, err := dist.LayerDiffID(bpLayerTar)
if err != nil {
return errors.Wrapf(err,
"getting content hashes for buildpack %s",
style.Symbol(bp.Descriptor().Info().FullName()),
)
}
collectionToAdd[bp.Descriptor().Info().FullName()] = toAdd{
tarPath: bpLayerTar,
diffID: diffID.String(),
Expand Down Expand Up @@ -278,19 +267,11 @@ func (b *PackageBuilder) finalizeExtensionImage(image WorkableImage, tmpDir stri
}

exLayers := dist.ModuleLayers{}
exLayerTar, err := ToLayerTar(tmpDir, b.extension)
diffID, exLayerTar, err := ToLayerTar(tmpDir, b.extension)
if err != nil {
return err
}

diffID, err := dist.LayerDiffID(exLayerTar)
if err != nil {
return errors.Wrapf(err,
"getting content hashes for extension %s",
style.Symbol(b.extension.Descriptor().Info().FullName()),
)
}

if err := image.AddLayerWithDiffID(exLayerTar, diffID.String()); err != nil {
return errors.Wrapf(err, "adding layer tar for extension %s", style.Symbol(b.extension.Descriptor().Info().FullName()))
}
Expand Down
43 changes: 31 additions & 12 deletions pkg/buildpack/buildpack.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,16 @@ package buildpack

import (
"archive/tar"
"crypto/sha256"
"encoding/hex"
"fmt"
"io"
"os"
"path"
"path/filepath"

v1 "github.com/google/go-containerregistry/pkg/v1"

"github.com/BurntSushi/toml"
"github.com/buildpacks/lifecycle/api"
"github.com/pkg/errors"
Expand Down Expand Up @@ -304,26 +308,41 @@ func validateExtensionDescriptor(extd dist.ExtensionDescriptor) error {
return nil
}

func ToLayerTar(dest string, module BuildModule) (string, error) {
descriptor := module.Descriptor()
func ToLayerTar(dest string, module BuildModule) (v1.Hash, string, error) {
modReader, err := module.Open()
if err != nil {
return "", errors.Wrap(err, "opening blob")
return v1.Hash{}, "", errors.Wrap(err, "opening blob")
}
defer modReader.Close()

layerTar := filepath.Join(dest, fmt.Sprintf("%s.%s.tar", descriptor.EscapedID(), descriptor.Info().Version))
fh, err := os.Create(layerTar)
if err != nil {
return "", errors.Wrap(err, "create file for tar")
}
defer fh.Close()
hasher := sha256.New()
var layerTar string
file, ok := modReader.(*os.File)
if ok {
layerTar = file.Name()
if _, err = io.Copy(hasher, modReader); err != nil {
return v1.Hash{}, "", errors.Wrap(err, "writing diffID")
}
} else {
descriptor := module.Descriptor()
layerTar = filepath.Join(dest, fmt.Sprintf("%s.%s.tar", descriptor.EscapedID(), descriptor.Info().Version))
fh, err := os.Create(layerTar)
if err != nil {
return v1.Hash{}, "", errors.Wrap(err, "create file for tar")
}
defer fh.Close()

if _, err := io.Copy(fh, modReader); err != nil {
return "", errors.Wrap(err, "writing blob to tar")
writer := io.MultiWriter(fh, hasher)

if _, err = io.Copy(writer, modReader); err != nil {
return v1.Hash{}, "", errors.Wrap(err, "writing blob to tar")
}
}

return layerTar, nil
return v1.Hash{
Algorithm: "sha256",
Hex: hex.EncodeToString(hasher.Sum(make([]byte, 0, hasher.Size()))),
}, layerTar, nil
}

// Set returns a set of the given string slice.
Expand Down
39 changes: 25 additions & 14 deletions pkg/buildpack/buildpack_tar_writer.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,17 @@ package buildpack

import (
"archive/tar"
"crypto/sha256"
"encoding/hex"
"fmt"
"io"
"os"
"path"
"path/filepath"
"strings"

v1 "github.com/google/go-containerregistry/pkg/v1"

"github.com/buildpacks/lifecycle/buildpack"
"github.com/pkg/errors"

Expand All @@ -34,17 +38,14 @@ func NewBuildModuleWriter(logger logging.Logger, factory archive.TarWriterFactor

// NToLayerTar creates a tar file containing the all the Buildpacks given, but excluding the ones which FullName() is
// in the exclude list. It returns the path to the tar file, the list of Buildpacks that were excluded, and any error
func (b *BuildModuleWriter) NToLayerTar(tarPath, filename string, modules []BuildModule, exclude map[string]struct{}) (string, []BuildModule, error) {
func (b *BuildModuleWriter) NToLayerTar(tarPath, filename string, modules []BuildModule, exclude map[string]struct{}) (v1.Hash, string, []BuildModule, error) {
layerTar := filepath.Join(tarPath, fmt.Sprintf("%s.tar", filename))
tarFile, err := os.Create(layerTar)
b.logger.Debugf("creating file %s", style.Symbol(layerTar))
if err != nil {
return "", nil, errors.Wrap(err, "create file for tar")
return v1.Hash{}, "", nil, errors.Wrap(err, "create file for tar")
}

defer tarFile.Close()
tw := b.factory.NewWriter(tarFile)
defer tw.Close()

parentFolderAdded := map[string]bool{}
duplicated := map[string]bool{}
Expand All @@ -56,8 +57,9 @@ func (b *BuildModuleWriter) NToLayerTar(tarPath, filename string, modules []Buil
duplicated[module.Descriptor().Info().FullName()] = true
b.logger.Debugf("adding %s", style.Symbol(module.Descriptor().Info().FullName()))

if err := b.writeBuildModuleToTar(tw, module, &parentFolderAdded); err != nil {
return "", nil, errors.Wrapf(err, "adding %s", style.Symbol(module.Descriptor().Info().FullName()))
err := b.writeBuildModuleToTar(tw, module, &parentFolderAdded)
if err != nil {
return v1.Hash{}, "", nil, errors.Wrapf(err, "adding %s", style.Symbol(module.Descriptor().Info().FullName()))
}
rootPath := processRootPath(module)
if !parentFolderAdded[rootPath] {
Expand All @@ -73,7 +75,21 @@ func (b *BuildModuleWriter) NToLayerTar(tarPath, filename string, modules []Buil
}

b.logger.Debugf("%s was created successfully", style.Symbol(layerTar))
return layerTar, buildModuleExcluded, nil
tarFile.Close()
tw.Close()

// calculate diffID
hasher := sha256.New()
if tarFile, err = os.Open(layerTar); err != nil {
return v1.Hash{}, "", nil, errors.Wrap(err, "open tar")
}
if _, err = io.Copy(hasher, tarFile); err != nil {
return v1.Hash{}, "", nil, errors.Wrap(err, "writing diffID")
}
return v1.Hash{
Algorithm: "sha256",
Hex: hex.EncodeToString(hasher.Sum(make([]byte, 0, hasher.Size()))),
}, layerTar, buildModuleExcluded, nil
}

// writeBuildModuleToTar writes the content of the given tar file into the writer, skipping the folders that were already added
Expand Down Expand Up @@ -109,12 +125,7 @@ func (b *BuildModuleWriter) writeBuildModuleToTar(tw archive.TarWriter, module B
return errors.Wrapf(err, "failed to write header for '%s'", header.Name)
}

buf, err := io.ReadAll(tr)
if err != nil {
return errors.Wrapf(err, "failed to read contents of '%s'", header.Name)
}

_, err = tw.Write(buf)
_, err = io.Copy(tw, tr)
if err != nil {
return errors.Wrapf(err, "failed to write contents to '%s'", header.Name)
}
Expand Down
8 changes: 4 additions & 4 deletions pkg/buildpack/buildpack_tar_writer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ func testBuildModuleWriter(t *testing.T, when spec.G, it spec.S) {
when("there are not duplicated buildpacks", func() {
it("creates a tar", func() {
bpModules := []buildpack.BuildModule{bp1v1, bp2v1, bp3v1}
tarFile, bpExcluded, err := buildModuleWriter.NToLayerTar(tmpDir, "test-file-1", bpModules, nil)
_, tarFile, bpExcluded, err := buildModuleWriter.NToLayerTar(tmpDir, "test-file-1", bpModules, nil)

h.AssertNil(t, err)
h.AssertTrue(t, len(bpExcluded) == 0)
Expand All @@ -122,7 +122,7 @@ func testBuildModuleWriter(t *testing.T, when spec.G, it spec.S) {
when("there are duplicated buildpacks", func() {
it("creates a tar skipping root folder from duplicated buildpacks", func() {
bpModules := []buildpack.BuildModule{bp1v1, bp1v2, bp2v1, bp3v1}
tarFile, bpExcluded, err := buildModuleWriter.NToLayerTar(tmpDir, "test-file-2", bpModules, nil)
_, tarFile, bpExcluded, err := buildModuleWriter.NToLayerTar(tmpDir, "test-file-2", bpModules, nil)

h.AssertNil(t, err)
h.AssertTrue(t, len(bpExcluded) == 0)
Expand All @@ -142,7 +142,7 @@ func testBuildModuleWriter(t *testing.T, when spec.G, it spec.S) {
when("there are not duplicated buildpacks", func() {
it("creates a tar skipping excluded buildpacks", func() {
bpModules := []buildpack.BuildModule{bp1v1, bp2v1, bp3v1}
tarFile, bpExcluded, err := buildModuleWriter.NToLayerTar(tmpDir, "test-file-3", bpModules, exclude)
_, tarFile, bpExcluded, err := buildModuleWriter.NToLayerTar(tmpDir, "test-file-3", bpModules, exclude)
h.AssertNil(t, err)
h.AssertTrue(t, len(bpExcluded) == 1)
h.AssertNotNil(t, tarFile)
Expand All @@ -154,7 +154,7 @@ func testBuildModuleWriter(t *testing.T, when spec.G, it spec.S) {
when("there are duplicated buildpacks", func() {
it("creates a tar skipping excluded buildpacks and root folder from duplicated buildpacks", func() {
bpModules := []buildpack.BuildModule{bp1v1, bp1v2, bp2v1, bp3v1}
tarFile, bpExcluded, err := buildModuleWriter.NToLayerTar(tmpDir, "test-file-4", bpModules, exclude)
_, tarFile, bpExcluded, err := buildModuleWriter.NToLayerTar(tmpDir, "test-file-4", bpModules, exclude)
h.AssertNil(t, err)
h.AssertTrue(t, len(bpExcluded) == 1)
h.AssertNotNil(t, tarFile)
Expand Down
9 changes: 0 additions & 9 deletions pkg/dist/layers.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
package dist

import (
"os"
"path/filepath"

"github.com/buildpacks/lifecycle/api"
v1 "github.com/google/go-containerregistry/pkg/v1"
"github.com/google/go-containerregistry/pkg/v1/tarball"
Expand All @@ -19,12 +16,6 @@ type Descriptor interface {
}

func LayerDiffID(layerTarPath string) (v1.Hash, error) {
fh, err := os.Open(filepath.Clean(layerTarPath))
if err != nil {
return v1.Hash{}, errors.Wrap(err, "opening tar file")
}
defer fh.Close()

layer, err := tarball.LayerFromFile(layerTarPath)
if err != nil {
return v1.Hash{}, errors.Wrap(err, "reading layer tar")
Expand Down