From a2cc1b1a201c5d312cb6cb167a3b5fcbf1e205d4 Mon Sep 17 00:00:00 2001
From: Yecheng Fu <fuyecheng@qiniu.com>
Date: Fri, 24 Aug 2018 16:42:27 +0800
Subject: [PATCH] Revert "Use sync.map to scale ecache better"

This reverts commit 17d0190706407bade8f6f875844749007904ad7d.
---
 pkg/scheduler/core/equivalence/eqivalence.go  | 39 ++++++++++++-------
 .../core/equivalence/eqivalence_test.go       |  2 +-
 2 files changed, 27 insertions(+), 14 deletions(-)

diff --git a/pkg/scheduler/core/equivalence/eqivalence.go b/pkg/scheduler/core/equivalence/eqivalence.go
index 115acc4d18..4ee59a3f35 100644
--- a/pkg/scheduler/core/equivalence/eqivalence.go
+++ b/pkg/scheduler/core/equivalence/eqivalence.go
@@ -35,6 +35,9 @@ import (
 	"github.com/golang/glog"
 )
 
+// nodeMap stores a *Cache for each node.
+type nodeMap map[string]*NodeCache
+
 // Cache is a thread safe map saves and reuses the output of predicate functions,
 // it uses node name as key to access those cached results.
 //
@@ -42,13 +45,17 @@ import (
 // class". (Equivalence class is defined in the `Class` type.) Saved results
 // will be reused until an appropriate invalidation function is called.
 type Cache struct {
-	// i.e. map[string]*NodeCache
-	sync.Map
+	// NOTE(harry): Theoretically sync.Map has better performance in machine with 8+ CPUs, while
+	// the reality is lock contention in first level cache is rare.
+	mu          sync.RWMutex
+	nodeToCache nodeMap
 }
 
 // NewCache create an empty equiv class cache.
 func NewCache() *Cache {
-	return new(Cache)
+	return &Cache{
+		nodeToCache: make(nodeMap),
+	}
 }
 
 // NodeCache saves and reuses the output of predicate functions. Use RunPredicate to
@@ -76,8 +83,12 @@ func newNodeCache() *NodeCache {
 // it creates the NodeCache and returns it.
 // The boolean flag is true if the value was loaded, false if created.
 func (c *Cache) GetNodeCache(name string) (nodeCache *NodeCache, exists bool) {
-	v, exists := c.LoadOrStore(name, newNodeCache())
-	nodeCache = v.(*NodeCache)
+	c.mu.Lock()
+	defer c.mu.Unlock()
+	if nodeCache, exists = c.nodeToCache[name]; !exists {
+		nodeCache = newNodeCache()
+		c.nodeToCache[name] = nodeCache
+	}
 	return
 }
 
@@ -86,13 +97,12 @@ func (c *Cache) InvalidatePredicates(predicateKeys sets.String) {
 	if len(predicateKeys) == 0 {
 		return
 	}
-	c.Range(func(k, v interface{}) bool {
-		n := v.(*NodeCache)
+	c.mu.RLock()
+	defer c.mu.RUnlock()
+	for _, n := range c.nodeToCache {
 		n.invalidatePreds(predicateKeys)
-		return true
-	})
+	}
 	glog.V(5).Infof("Cache invalidation: node=*,predicates=%v", predicateKeys)
-
 }
 
 // InvalidatePredicatesOnNode clears cached results for the given predicates on one node.
@@ -100,8 +110,9 @@ func (c *Cache) InvalidatePredicatesOnNode(nodeName string, predicateKeys sets.S
 	if len(predicateKeys) == 0 {
 		return
 	}
-	if v, ok := c.Load(nodeName); ok {
-		n := v.(*NodeCache)
+	c.mu.RLock()
+	defer c.mu.RUnlock()
+	if n, ok := c.nodeToCache[nodeName]; ok {
 		n.invalidatePreds(predicateKeys)
 	}
 	glog.V(5).Infof("Cache invalidation: node=%s,predicates=%v", nodeName, predicateKeys)
@@ -109,7 +120,9 @@ func (c *Cache) InvalidatePredicatesOnNode(nodeName string, predicateKeys sets.S
 
 // InvalidateAllPredicatesOnNode clears all cached results for one node.
 func (c *Cache) InvalidateAllPredicatesOnNode(nodeName string) {
-	c.Delete(nodeName)
+	c.mu.Lock()
+	defer c.mu.Unlock()
+	delete(c.nodeToCache, nodeName)
 	glog.V(5).Infof("Cache invalidation: node=%s,predicates=*", nodeName)
 }
 
diff --git a/pkg/scheduler/core/equivalence/eqivalence_test.go b/pkg/scheduler/core/equivalence/eqivalence_test.go
index e807958cc4..47e1d95774 100644
--- a/pkg/scheduler/core/equivalence/eqivalence_test.go
+++ b/pkg/scheduler/core/equivalence/eqivalence_test.go
@@ -731,7 +731,7 @@ func TestInvalidateCachedPredicateItemOfAllNodes(t *testing.T) {
 	// there should be no cached predicate any more
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
-			if nodeCache, exist := ecache.GetNodeCache(test.nodeName); exist {
+			if nodeCache, exist := ecache.nodeToCache[test.nodeName]; exist {
 				if _, exist := nodeCache.cache[testPredicate]; exist {
 					t.Errorf("Failed: cached item for predicate key: %v on node: %v should be invalidated",
 						testPredicate, test.nodeName)