From f745869fbc766af462baad554d080f51440a96d5 Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Tue, 19 Oct 2021 14:45:35 +0200 Subject: [PATCH] libimage: speed up image filters With commit 86afb94e0054 the dangling checks have been changed to be compatible with Docker. Since then, the dangling also need to compute children. Speed up the dangling and intermediate checks by computing the layer tree *once* instead of for each filter invocation. **Before:** real 0m10.837s user 0m11.308s sys 0m4.231s **After:** real 0m0.476s user 0m0.478s sys 0m0.151s Context: github.com/containers/podman/issues/11997 Signed-off-by: Valentin Rothberg --- libimage/filters.go | 32 ++++++++++++++++++++++++++------ libimage/image.go | 36 ++++++++++++++++++++++++++---------- 2 files changed, 52 insertions(+), 16 deletions(-) diff --git a/libimage/filters.go b/libimage/filters.go index 833f940cc..521af5d06 100644 --- a/libimage/filters.go +++ b/libimage/filters.go @@ -50,6 +50,18 @@ func filterImages(images []*Image, filters []filterFunc) ([]*Image, error) { func (r *Runtime) compileImageFilters(ctx context.Context, options *ListImagesOptions) ([]filterFunc, error) { logrus.Tracef("Parsing image filters %s", options.Filters) + var tree *layerTree + getTree := func() (*layerTree, error) { + if tree == nil { + t, err := r.layerTree() + if err != nil { + return nil, err + } + tree = t + } + return tree, nil + } + filterFuncs := []filterFunc{} for _, filter := range options.Filters { var key, value string @@ -93,7 +105,11 @@ func (r *Runtime) compileImageFilters(ctx context.Context, options *ListImagesOp if err != nil { return nil, errors.Wrapf(err, "non-boolean value %q for dangling filter", value) } - filterFuncs = append(filterFuncs, filterDangling(ctx, dangling)) + t, err := getTree() + if err != nil { + return nil, err + } + filterFuncs = append(filterFuncs, filterDangling(ctx, dangling, t)) case "id": filterFuncs = append(filterFuncs, filterID(value)) @@ -103,7 +119,11 @@ func (r *Runtime) compileImageFilters(ctx context.Context, options *ListImagesOp if err != nil { return nil, errors.Wrapf(err, "non-boolean value %q for intermediate filter", value) } - filterFuncs = append(filterFuncs, filterIntermediate(ctx, intermediate)) + t, err := getTree() + if err != nil { + return nil, err + } + filterFuncs = append(filterFuncs, filterIntermediate(ctx, intermediate, t)) case "label": filterFuncs = append(filterFuncs, filterLabel(ctx, value)) @@ -221,9 +241,9 @@ func filterContainers(value string, fn IsExternalContainerFunc) filterFunc { } // filterDangling creates a dangling filter for matching the specified value. -func filterDangling(ctx context.Context, value bool) filterFunc { +func filterDangling(ctx context.Context, value bool, tree *layerTree) filterFunc { return func(img *Image) (bool, error) { - isDangling, err := img.IsDangling(ctx) + isDangling, err := img.isDangling(ctx, tree) if err != nil { return false, err } @@ -241,9 +261,9 @@ func filterID(value string) filterFunc { // filterIntermediate creates an intermediate filter for images. An image is // considered to be an intermediate image if it is dangling (i.e., no tags) and // has no children (i.e., no other image depends on it). -func filterIntermediate(ctx context.Context, value bool) filterFunc { +func filterIntermediate(ctx context.Context, value bool, tree *layerTree) filterFunc { return func(img *Image) (bool, error) { - isIntermediate, err := img.IsIntermediate(ctx) + isIntermediate, err := img.isIntermediate(ctx, tree) if err != nil { return false, err } diff --git a/libimage/image.go b/libimage/image.go index 00a2d620e..bf3310da2 100644 --- a/libimage/image.go +++ b/libimage/image.go @@ -128,10 +128,16 @@ func (i *Image) IsReadOnly() bool { // IsDangling returns true if the image is dangling, that is an untagged image // without children. func (i *Image) IsDangling(ctx context.Context) (bool, error) { + return i.isDangling(ctx, nil) +} + +// isDangling returns true if the image is dangling, that is an untagged image +// without children. If tree is nil, it will created for this invocation only. +func (i *Image) isDangling(ctx context.Context, tree *layerTree) (bool, error) { if len(i.Names()) > 0 { return false, nil } - children, err := i.getChildren(ctx, false) + children, err := i.getChildren(ctx, false, tree) if err != nil { return false, err } @@ -141,10 +147,17 @@ func (i *Image) IsDangling(ctx context.Context) (bool, error) { // IsIntermediate returns true if the image is an intermediate image, that is // an untagged image with children. func (i *Image) IsIntermediate(ctx context.Context) (bool, error) { + return i.isIntermediate(ctx, nil) +} + +// isIntermediate returns true if the image is an intermediate image, that is +// an untagged image with children. If tree is nil, it will created for this +// invocation only. +func (i *Image) isIntermediate(ctx context.Context, tree *layerTree) (bool, error) { if len(i.Names()) > 0 { return false, nil } - children, err := i.getChildren(ctx, false) + children, err := i.getChildren(ctx, false, tree) if err != nil { return false, err } @@ -189,7 +202,7 @@ func (i *Image) Parent(ctx context.Context) (*Image, error) { // HasChildren returns indicates if the image has children. func (i *Image) HasChildren(ctx context.Context) (bool, error) { - children, err := i.getChildren(ctx, false) + children, err := i.getChildren(ctx, false, nil) if err != nil { return false, err } @@ -198,7 +211,7 @@ func (i *Image) HasChildren(ctx context.Context) (bool, error) { // Children returns the image's children. func (i *Image) Children(ctx context.Context) ([]*Image, error) { - children, err := i.getChildren(ctx, true) + children, err := i.getChildren(ctx, true, nil) if err != nil { return nil, err } @@ -206,13 +219,16 @@ func (i *Image) Children(ctx context.Context) ([]*Image, error) { } // getChildren returns a list of imageIDs that depend on the image. If all is -// false, only the first child image is returned. -func (i *Image) getChildren(ctx context.Context, all bool) ([]*Image, error) { - tree, err := i.runtime.layerTree() - if err != nil { - return nil, err +// false, only the first child image is returned. If tree is nil, it will be +// created for this invocation only. +func (i *Image) getChildren(ctx context.Context, all bool, tree *layerTree) ([]*Image, error) { + if tree == nil { + t, err := i.runtime.layerTree() + if err != nil { + return nil, err + } + tree = t } - return tree.children(ctx, i, all) }