feat: use atomic pointer for global feature flag client

[cbosss]

Use an rwmutex atomic pointer when accessing the global featureflag client.

[paulo]

Nice!

[mraerino]

this is good, but we should replace it with an atomic ptr in the future.
we also need to improve testability of featureflags which is kinda messy right now.

[mrdg]

I think the only reason there's a lock here is because there's a SetGlobalClient function, which is mostly used in tests it seems. Probably makes sense to solve that some other way and get rid of the lock altogether.

[cbosss]

Did a quick benchmark:

var globalLock sync.RWMutex
var globalClient Client = MockClient{}

// See https://blog.dubbelboer.com/2015/08/23/rwmutex-vs-atomicvalue-vs-unsafepointer.html
var globalLockAtomic = atomic.NewUnsafePointer(unsafe.Pointer(&globalClient))

func SetGlobalClient(client Client) {
	if client == nil {
		return
	}
	globalLock.Lock()
	globalClient = client
	globalLock.Unlock()

	globalLockAtomic.Store(unsafe.Pointer(&client))
}

func GetGlobalClient() Client {
	globalLock.RLock()
	defer globalLock.RUnlock()
	return globalClient
}

func GetGlobalClientAtomic() Client {
	c := (*Client)(globalLockAtomic.Load())
	return *c
}

func BenchmarkGlobalAccess(b *testing.B) {
	SetGlobalClient(&ldClient{})

	var c1, c2 Client

	b.Run("rwmutex", func(b *testing.B) {
		b.ReportAllocs()
		for i := 0; i < b.N; i++ {
			c1 = GetGlobalClient()
		}
	})

	b.Run("atomic", func(b *testing.B) {
		b.ReportAllocs()
		for i := 0; i < b.N; i++ {
			c2 = GetGlobalClientAtomic()
		}
	})
	require.Equal(b, c1, c2)
}

goos: darwin
goarch: amd64
pkg: github.com/netlify/netlify-commons/featureflag
cpu: Intel(R) Core(TM) i7-1068NG7 CPU @ 2.30GHz
BenchmarkGlobalAccess
BenchmarkGlobalAccess/rwmutex
BenchmarkGlobalAccess/rwmutex-8         	76735899	        15.40 ns/op	       0 B/op	       0 allocs/op
BenchmarkGlobalAccess/atomic
BenchmarkGlobalAccess/atomic-8          	1000000000	         0.8881 ns/op	       0 B/op	       0 allocs/op
PASS

Take this with a grain of salt, because there's no contention.

[mraerino]

i don't think the dependency on the atomic lib from uber is needed but i don't care a lot

[mraerino]

@cbosss wanna ship this?