graph: add parent img refcount for faster rmi

also fix a typo in pkg/truncindex package comment

Signed-off-by: Antonio Murdaca <runcom@redhat.com>

daemon/daemonbuilder/builder.go

@@ -193,7 +193,7 @@ func (d Docker) Copy(c *daemon.Container, destPath string, src builder.FileInfo,
 // GetCachedImage returns a reference to a cached image whose parent equals `parent`
 // and runconfig equals `cfg`. A cache miss is expected to return an empty ID and a nil error.
 func (d Docker) GetCachedImage(imgID string, cfg *runconfig.Config) (string, error) {
-	cache, err := d.Daemon.ImageGetCached(string(imgID), cfg)
+	cache, err := d.Daemon.ImageGetCached(imgID, cfg)
 	if cache == nil || err != nil {
 		return "", err
 	}

daemon/image_delete.go

@@ -279,7 +279,7 @@ func (daemon *Daemon) checkImageDeleteHardConflict(img *image.Image) *imageDelet
 	}
 
 	// Check if the image has any descendent images.
-	if daemon.Graph().HasChildren(img) {
+	if daemon.Graph().HasChildren(img.ID) {
 		return &imageDeleteConflict{
 			hard:    true,
 			imgID:   img.ID,
@@ -337,5 +337,5 @@ func (daemon *Daemon) checkImageDeleteSoftConflict(img *image.Image) *imageDelet
 // that there are no repository references to the given image and it has no
 // child images.
 func (daemon *Daemon) imageIsDangling(img *image.Image) bool {
-	return !(daemon.repositories.HasReferences(img) || daemon.Graph().HasChildren(img))
+	return !(daemon.Repositories().HasReferences(img) || daemon.Graph().HasChildren(img.ID))
 }

graph/graph.go

@@ -104,6 +104,7 @@ type Graph struct {
 	tarSplitDisabled bool
 	uidMaps          []idtools.IDMap
 	gidMaps          []idtools.IDMap
+	parentRefs       map[string]int
 }
 
 // file names for ./graph/<ID>/
@@ -141,12 +142,13 @@ func NewGraph(root string, driver graphdriver.Driver, uidMaps, gidMaps []idtools
 	}
 
 	graph := &Graph{
-		root:     abspath,
-		idIndex:  truncindex.NewTruncIndex([]string{}),
-		driver:   driver,
-		retained: &retainedLayers{layerHolders: make(map[string]map[string]struct{})},
-		uidMaps:  uidMaps,
-		gidMaps:  gidMaps,
+		root:       abspath,
+		idIndex:    truncindex.NewTruncIndex([]string{}),
+		driver:     driver,
+		retained:   &retainedLayers{layerHolders: make(map[string]map[string]struct{})},
+		uidMaps:    uidMaps,
+		gidMaps:    gidMaps,
+		parentRefs: make(map[string]int),
 	}
 
 	// Windows does not currently support tarsplit functionality.
@@ -174,6 +176,20 @@ func (graph *Graph) restore() error {
 	for _, v := range dir {
 		id := v.Name()
 		if graph.driver.Exists(id) {
+			pth := filepath.Join(graph.root, id, "json")
+			jsonSource, err := os.Open(pth)
+			if err != nil {
+				return err
+			}
+			defer jsonSource.Close()
+			decoder := json.NewDecoder(jsonSource)
+			var img *image.Image
+			if err := decoder.Decode(img); err != nil {
+				return err
+			}
+			graph.imageMutex.Lock(img.Parent)
+			graph.parentRefs[img.Parent]++
+			graph.imageMutex.Unlock(img.Parent)
 			ids = append(ids, id)
 		}
 	}
@@ -326,7 +342,13 @@ func (graph *Graph) register(im image.Descriptor, layerData io.Reader) (err erro
 	if err := os.Rename(tmp, graph.imageRoot(imgID)); err != nil {
 		return err
 	}
-	graph.idIndex.Add(imgID)
+
+	graph.idIndex.Add(img.ID)
+
+	graph.imageMutex.Lock(img.Parent)
+	graph.parentRefs[img.Parent]++
+	graph.imageMutex.Unlock(img.Parent)
+
 	return nil
 }
 
@@ -393,6 +415,10 @@ func (graph *Graph) Delete(name string) error {
 	if err != nil {
 		return err
 	}
+	img, err := graph.Get(id)
+	if err != nil {
+		return err
+	}
 	tmp, err := graph.mktemp()
 	graph.idIndex.Delete(id)
 	if err == nil {
@@ -407,6 +433,14 @@ func (graph *Graph) Delete(name string) error {
 	}
 	// Remove rootfs data from the driver
 	graph.driver.Remove(id)
+
+	graph.imageMutex.Lock(img.Parent)
+	graph.parentRefs[img.Parent]--
+	if graph.parentRefs[img.Parent] == 0 {
+		delete(graph.parentRefs, img.Parent)
+	}
+	graph.imageMutex.Unlock(img.Parent)
+
 	// Remove the trashed image directory
 	return os.RemoveAll(tmp)
 }
@@ -424,9 +458,11 @@ func (graph *Graph) Map() map[string]*image.Image {
 // The walking order is undetermined.
 func (graph *Graph) walkAll(handler func(*image.Image)) {
 	graph.idIndex.Iterate(func(id string) {
-		if img, err := graph.Get(id); err != nil {
+		img, err := graph.Get(id)
+		if err != nil {
 			return
-		} else if handler != nil {
+		}
+		if handler != nil {
 			handler(img)
 		}
 	})
@@ -453,8 +489,11 @@ func (graph *Graph) ByParent() map[string][]*image.Image {
 }
 
 // HasChildren returns whether the given image has any child images.
-func (graph *Graph) HasChildren(img *image.Image) bool {
-	return len(graph.ByParent()[img.ID]) > 0
+func (graph *Graph) HasChildren(imgID string) bool {
+	graph.imageMutex.Lock(imgID)
+	count := graph.parentRefs[imgID]
+	graph.imageMutex.Unlock(imgID)
+	return count > 0
 }
 
 // Retain keeps the images and layers that are in the pulling chain so that
@@ -472,11 +511,10 @@ func (graph *Graph) Release(sessionID string, layerIDs ...string) {
 // A head is an image which is not the parent of another image in the graph.
 func (graph *Graph) Heads() map[string]*image.Image {
 	heads := make(map[string]*image.Image)
-	byParent := graph.ByParent()
 	graph.walkAll(func(image *image.Image) {
 		// If it's not in the byParent lookup table, then
 		// it's not a parent -> so it's a head!
-		if _, exists := byParent[image.ID]; !exists {
+		if !graph.HasChildren(image.ID) {
 			heads[image.ID] = image
 		}
 	})

integration-cli/docker_cli_images_test.go

@@ -51,7 +51,6 @@ func (s *DockerSuite) TestImagesEnsureImageWithBadTagIsNotListed(c *check.C) {
 	if strings.Contains(out, "busybox") {
 		c.Fatal("images should not have listed busybox")
 	}
-
 }
 
 func (s *DockerSuite) TestImagesOrderedByCreationDate(c *check.C) {
@@ -204,3 +203,42 @@ func (s *DockerSuite) TestImagesWithIncorrectFilter(c *check.C) {
 	c.Assert(err, check.NotNil)
 	c.Assert(out, checker.Contains, "Invalid filter")
 }
+
+func (s *DockerSuite) TestImagesEnsureOnlyHeadsImagesShown(c *check.C) {
+	testRequires(c, DaemonIsLinux)
+
+	dockerfile := `
+        FROM scratch
+        MAINTAINER docker
+        ENV foo bar`
+
+	head, out, err := buildImageWithOut("scratch-image", dockerfile, false)
+	c.Assert(err, check.IsNil)
+
+	split := strings.Split(out, "\n")
+	intermediate := strings.TrimSpace(split[5][7:])
+
+	out, _ = dockerCmd(c, "images")
+	if strings.Contains(out, intermediate) {
+		c.Fatalf("images shouldn't show non-heads images, got %s in %s", intermediate, out)
+	}
+	if !strings.Contains(out, head[:12]) {
+		c.Fatalf("images should contain final built images, want %s in out, got %s", head[:12], out)
+	}
+}
+
+func (s *DockerSuite) TestImagesEnsureImagesFromScratchShown(c *check.C) {
+	testRequires(c, DaemonIsLinux)
+
+	dockerfile := `
+        FROM scratch
+        MAINTAINER docker`
+
+	id, _, err := buildImageWithOut("scratch-image", dockerfile, false)
+	c.Assert(err, check.IsNil)
+
+	out, _ := dockerCmd(c, "images")
+	if !strings.Contains(out, id[:12]) {
+		c.Fatalf("images should contain images built from scratch (e.g. %s), got %s", id[:12], out)
+	}
+}

integration-cli/docker_cli_rmi_test.go

@@ -284,3 +284,15 @@ RUN echo 2 #layer2
 	// should be allowed to untag with the -f flag
 	c.Assert(out, checker.Contains, fmt.Sprintf("Untagged: %s:latest", newTag))
 }
+
+func (*DockerSuite) TestRmiParentImageFail(c *check.C) {
+	testRequires(c, DaemonIsLinux)
+
+	parent, err := inspectField("busybox", "Parent")
+	c.Assert(err, check.IsNil)
+	out, _, err := dockerCmdWithError("rmi", parent)
+	c.Assert(err, check.NotNil)
+	if !strings.Contains(out, "image has dependent child images") {
+		c.Fatalf("rmi should have failed because it's a parent image, got %s", out)
+	}
+}