From 05250a4f0094e6802dd7d338d632ea632d0c7e34 Mon Sep 17 00:00:00 2001
From: Xinfeng Liu <xinfeng.liu@gmail.com>
Date: Tue, 23 Apr 2019 02:33:20 +0000
Subject: [PATCH] layer: optimize layerStore mountL

Goroutine stack analisys shown some lock contention
while doing massively (100 instances of `docker rm`)
parallel image removal, with many goroutines waiting
for the mountL mutex. Optimize it.

With this commit, the above operation is about 3x
faster, with no noticeable change to container
creation times (tested on aufs and overlay2).

kolyshkin@:
- squashed commits
- added description
- protected CreateRWLayer against name collisions by
temporary assiging nil to ls.mounts[name], and treating
nil as "non-existent" in all the other functions.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
---
 layer/layer_store.go | 50 ++++++++++++++++++++++++++++----------------
 layer/migration.go   |  6 +++---
 2 files changed, 35 insertions(+), 21 deletions(-)

diff --git a/layer/layer_store.go b/layer/layer_store.go
index 1601465c04..737283cc2d 100644
--- a/layer/layer_store.go
+++ b/layer/layer_store.go
@@ -189,7 +189,9 @@ func (ls *layerStore) loadLayer(layer ChainID) (*roLayer, error) {
 }
 
 func (ls *layerStore) loadMount(mount string) error {
-	if _, ok := ls.mounts[mount]; ok {
+	ls.mountL.Lock()
+	defer ls.mountL.Unlock()
+	if m := ls.mounts[mount]; m != nil {
 		return nil
 	}
 
@@ -477,7 +479,7 @@ func (ls *layerStore) Release(l Layer) ([]Metadata, error) {
 	return ls.releaseLayer(layer)
 }
 
-func (ls *layerStore) CreateRWLayer(name string, parent ChainID, opts *CreateRWLayerOpts) (RWLayer, error) {
+func (ls *layerStore) CreateRWLayer(name string, parent ChainID, opts *CreateRWLayerOpts) (_ RWLayer, err error) {
 	var (
 		storageOpt map[string]string
 		initFunc   MountInit
@@ -491,13 +493,21 @@ func (ls *layerStore) CreateRWLayer(name string, parent ChainID, opts *CreateRWL
 	}
 
 	ls.mountL.Lock()
-	defer ls.mountL.Unlock()
-	m, ok := ls.mounts[name]
-	if ok {
+	if _, ok := ls.mounts[name]; ok {
+		ls.mountL.Unlock()
 		return nil, ErrMountNameConflict
 	}
+	// Avoid name collision by temporary assigning nil
+	ls.mounts[name] = nil
+	ls.mountL.Unlock()
+	defer func() {
+		if err != nil {
+			ls.mountL.Lock()
+			delete(ls.mounts, name)
+			ls.mountL.Unlock()
+		}
+	}()
 
-	var err error
 	var pid string
 	var p *roLayer
 	if string(parent) != "" {
@@ -517,7 +527,7 @@ func (ls *layerStore) CreateRWLayer(name string, parent ChainID, opts *CreateRWL
 		}()
 	}
 
-	m = &mountedLayer{
+	m := &mountedLayer{
 		name:       name,
 		parent:     p,
 		mountID:    ls.mountID(name),
@@ -528,7 +538,7 @@ func (ls *layerStore) CreateRWLayer(name string, parent ChainID, opts *CreateRWL
 	if initFunc != nil {
 		pid, err = ls.initMount(m.mountID, pid, mountLabel, initFunc, storageOpt)
 		if err != nil {
-			return nil, err
+			return
 		}
 		m.initID = pid
 	}
@@ -538,10 +548,10 @@ func (ls *layerStore) CreateRWLayer(name string, parent ChainID, opts *CreateRWL
 	}
 
 	if err = ls.driver.CreateReadWrite(m.mountID, pid, createOpts); err != nil {
-		return nil, err
+		return
 	}
 	if err = ls.saveMount(m); err != nil {
-		return nil, err
+		return
 	}
 
 	return m.getReference(), nil
@@ -549,9 +559,9 @@ func (ls *layerStore) CreateRWLayer(name string, parent ChainID, opts *CreateRWL
 
 func (ls *layerStore) GetRWLayer(id string) (RWLayer, error) {
 	ls.mountL.Lock()
-	defer ls.mountL.Unlock()
-	mount, ok := ls.mounts[id]
-	if !ok {
+	mount := ls.mounts[id]
+	ls.mountL.Unlock()
+	if mount == nil {
 		return nil, ErrMountDoesNotExist
 	}
 
@@ -561,8 +571,8 @@ func (ls *layerStore) GetRWLayer(id string) (RWLayer, error) {
 func (ls *layerStore) GetMountID(id string) (string, error) {
 	ls.mountL.Lock()
 	defer ls.mountL.Unlock()
-	mount, ok := ls.mounts[id]
-	if !ok {
+	mount := ls.mounts[id]
+	if mount == nil {
 		return "", ErrMountDoesNotExist
 	}
 	logrus.Debugf("GetMountID id: %s -> mountID: %s", id, mount.mountID)
@@ -572,9 +582,9 @@ func (ls *layerStore) GetMountID(id string) (string, error) {
 
 func (ls *layerStore) ReleaseRWLayer(l RWLayer) ([]Metadata, error) {
 	ls.mountL.Lock()
-	defer ls.mountL.Unlock()
-	m, ok := ls.mounts[l.Name()]
-	if !ok {
+	m := ls.mounts[l.Name()]
+	ls.mountL.Unlock()
+	if m == nil {
 		return []Metadata{}, nil
 	}
 
@@ -606,7 +616,9 @@ func (ls *layerStore) ReleaseRWLayer(l RWLayer) ([]Metadata, error) {
 		return nil, err
 	}
 
+	ls.mountL.Lock()
 	delete(ls.mounts, m.Name())
+	ls.mountL.Unlock()
 
 	ls.layerL.Lock()
 	defer ls.layerL.Unlock()
@@ -634,7 +646,9 @@ func (ls *layerStore) saveMount(mount *mountedLayer) error {
 		}
 	}
 
+	ls.mountL.Lock()
 	ls.mounts[mount.name] = mount
+	ls.mountL.Unlock()
 
 	return nil
 }
diff --git a/layer/migration.go b/layer/migration.go
index 2668ea96bb..775e552fee 100644
--- a/layer/migration.go
+++ b/layer/migration.go
@@ -18,9 +18,9 @@ import (
 // after migration the layer may be retrieved by the given name.
 func (ls *layerStore) CreateRWLayerByGraphID(name, graphID string, parent ChainID) (err error) {
 	ls.mountL.Lock()
-	defer ls.mountL.Unlock()
-	m, ok := ls.mounts[name]
-	if ok {
+	m := ls.mounts[name]
+	ls.mountL.Unlock()
+	if m != nil {
 		if m.parent.chainID != parent {
 			return errors.New("name conflict, mismatched parent")
 		}