[FAB-11135] improving certpool peformance

Improved performance by loading system cert pool only once.
Split Get(...certs) to separate function (Add & Get) for
 adding certs to certpool and getting updated certpool.

go test -bench=.
goos: linux
goarch: amd64
pkg: github.com/hyperledger/fabric-sdk-go/pkg/core/config/comm/tls
BenchmarkTLSCertPool-6                	50000000	        20.5 ns/op
BenchmarkTLSCertPoolSameCert-6        	 5000000	       264 ns/op
BenchmarkTLSCertPoolDifferentCert-6   	  500000	      3223 ns/op
PASS
ok  	github.com/hyperledger/fabric-sdk-go/pkg/core/config/comm/tls	9.902s



Change-Id: Icb98a463de05885830efad9ff72a686e025e5e6a
Signed-off-by: Sudesh Shetty <[email protected]>

pkg/common/providers/fab/provider.go

@@ -161,5 +161,8 @@ type Providers interface {
 // cert pool implementation.
 type CertPool interface {
 	// Get returns the cert pool, optionally adding the provided certs
-	Get(certs ...*x509.Certificate) (*x509.CertPool, error)
+	Get() (*x509.CertPool, error)
+	//Add allows adding certificates to CertPool
+	//Call Get() after Add() to get the updated certpool
+	Add(certs ...*x509.Certificate)
 }

pkg/common/providers/test/mockfab/mockconfig.go

@@ -73,6 +73,11 @@ type MockCertPool struct {
 }
 
 //Get mock implementation of fab CertPool.Get()
-func (c *MockCertPool) Get(certs ...*x509.Certificate) (*x509.CertPool, error) {
+func (c *MockCertPool) Get() (*x509.CertPool, error) {
 	return c.CertPool, c.Err
 }
+
+//Add mock impl of adding certs to cert pool queue
+func (c *MockCertPool) Add(certs ...*x509.Certificate) {
+
+}

pkg/core/config/comm/comm.go

@@ -19,16 +19,14 @@ import (
 // certs for mutual TLS, and server host override. Works with certs loaded either from a path or embedded pem.
 func TLSConfig(cert *x509.Certificate, serverName string, config fab.EndpointConfig) (*tls.Config, error) {
 
-	certPool, err := config.TLSCACertPool().Get(cert)
-	if err != nil {
-		return nil, err
+	if cert != nil {
+		config.TLSCACertPool().Add(cert)
 	}
 
-	if certPool == nil || len(certPool.Subjects()) == 0 {
-		//Return empty tls config if certpool is unavailable
-		return &tls.Config{}, nil
+	certPool, err := config.TLSCACertPool().Get()
+	if err != nil {
+		return nil, err
 	}
-
 	return &tls.Config{RootCAs: certPool, Certificates: config.TLSClientCerts(), ServerName: serverName}, nil
 }
 

pkg/core/config/comm/tls/certpool.go

@@ -10,6 +10,8 @@ import (
 	"crypto/x509"
 	"sync"
 
+	"sync/atomic"
+
 	"github.com/hyperledger/fabric-sdk-go/pkg/common/logging"
 	"github.com/hyperledger/fabric-sdk-go/pkg/common/providers/fab"
 )
@@ -20,72 +22,114 @@ var logger = logging.NewLogger("fabsdk/core")
 // cert pool implementation.
 // It optionally allows loading the system trust store.
 type certPool struct {
-	useSystemCertPool bool
-	certs             []*x509.Certificate
-	certsByName       map[string][]int
-	lock              sync.RWMutex
+	certPool    *x509.CertPool
+	certs       []*x509.Certificate
+	certsByName map[string][]int
+	lock        sync.Mutex
+	dirty       int32
+	certQueue   []*x509.Certificate
 }
 
 // NewCertPool new CertPool implementation
-func NewCertPool(useSystemCertPool bool) fab.CertPool {
-	return &certPool{
-		useSystemCertPool: useSystemCertPool,
-		certsByName:       make(map[string][]int),
+func NewCertPool(useSystemCertPool bool) (fab.CertPool, error) {
+
+	c, err := loadSystemCertPool(useSystemCertPool)
+	if err != nil {
+		return nil, err
+	}
+
+	newCertPool := &certPool{
+		certsByName: make(map[string][]int),
+		certPool:    c,
 	}
+
+	return newCertPool, nil
 }
 
-func (c *certPool) Get(certs ...*x509.Certificate) (*x509.CertPool, error) {
+//Get returns certpool
+//if there are any certs in cert queue added by any previous Add() call, it adds those certs to certpool before returning
+func (c *certPool) Get() (*x509.CertPool, error) {
+
+	//if dirty then add certs from queue to cert pool
+	if atomic.CompareAndSwapInt32(&c.dirty, 1, 0) {
 
-	if len(certs) > 0 {
 		c.lock.Lock()
-		//add certs to SDK cert list
-		for _, newCert := range certs {
-			c.addCert(newCert)
+		defer c.lock.Unlock()
+
+		//add all new certs in queue to cert pool
+		for _, cert := range c.certQueue {
+			c.certPool.AddCert(cert)
 		}
-		c.lock.Unlock()
+		c.certQueue = []*x509.Certificate{}
 	}
 
-	c.lock.RLock()
-	defer c.lock.RUnlock()
+	return c.certPool, nil
+}
 
-	// create the cert pool
-	certPool, err := c.loadSystemCertPool()
-	if err != nil {
-		return nil, err
+//Add adds given certs to cert pool queue, those certs will be added to certpool during subsequent Get() call
+func (c *certPool) Add(certs ...*x509.Certificate) {
+	if len(certs) == 0 {
+		return
 	}
 
-	//add all certs to cert pool
-	for _, cert := range c.certs {
-		certPool.AddCert(cert)
-	}
+	c.lock.Lock()
+	defer c.lock.Unlock()
 
-	return certPool, nil
-}
+	//filter certs to be added, check if they already exist or duplicate
+	certsToBeAdded := c.filterCerts(certs...)
+
+	if len(certsToBeAdded) > 0 {
 
-func (c *certPool) addCert(newCert *x509.Certificate) {
-	if newCert != nil && !c.containsCert(newCert) {
-		n := len(c.certs)
-		// Store cert
-		c.certs = append(c.certs, newCert)
-		// Store cert name index
-		name := string(newCert.RawSubject)
-		c.certsByName[name] = append(c.certsByName[name], n)
+		for _, newCert := range certsToBeAdded {
+			c.certQueue = append(c.certQueue, newCert)
+			// Store cert name index
+			name := string(newCert.RawSubject)
+			c.certsByName[name] = append(c.certsByName[name], len(c.certs))
+			// Store cert
+			c.certs = append(c.certs, newCert)
+		}
+
+		atomic.CompareAndSwapInt32(&c.dirty, 0, 1)
 	}
 }
 
-func (c *certPool) containsCert(newCert *x509.Certificate) bool {
-	possibilities := c.certsByName[string(newCert.RawSubject)]
-	for _, p := range possibilities {
-		if c.certs[p].Equal(newCert) {
-			return true
+//filterCerts remove certs from list if they already exist in pool or duplicate
+func (c *certPool) filterCerts(certs ...*x509.Certificate) []*x509.Certificate {
+	filtered := []*x509.Certificate{}
+
+CertLoop:
+	for _, cert := range certs {
+		if cert == nil {
+			continue
 		}
+		possibilities := c.certsByName[string(cert.RawSubject)]
+		for _, p := range possibilities {
+			if c.certs[p].Equal(cert) {
+				continue CertLoop
+			}
+		}
+		filtered = append(filtered, cert)
 	}
 
-	return false
+	//remove duplicate from list of certs being passed
+	return removeDuplicates(filtered...)
+}
+
+func removeDuplicates(certs ...*x509.Certificate) []*x509.Certificate {
+	encountered := map[*x509.Certificate]bool{}
+	result := []*x509.Certificate{}
+
+	for v := range certs {
+		if !encountered[certs[v]] {
+			encountered[certs[v]] = true
+			result = append(result, certs[v])
+		}
+	}
+	return result
 }
 
-func (c *certPool) loadSystemCertPool() (*x509.CertPool, error) {
-	if !c.useSystemCertPool {
+func loadSystemCertPool(useSystemCertPool bool) (*x509.CertPool, error) {
+	if !useSystemCertPool {
 		return x509.NewCertPool(), nil
 	}
 	systemCertPool, err := x509.SystemCertPool()