snapshot: use per-Snapshot PRNGs to reduce lock contention + cleanup

snapshot/nil.go

@@ -1,6 +1,7 @@
 package snapshot
 
 import (
+	"math/rand"
 	"time"
 
 	"github.com/lyft/goruntime/snapshot/entry"
@@ -10,13 +11,15 @@ import (
 type Nil struct{}
 
 func NewNil() (s *Nil) {
-	s = &Nil{}
+	return &Nil{}
+}
 
-	return
+var nilRandom = random{
+	rr: rand.New(rand.NewSource(time.Now().UnixNano())),
 }
 
 func (n *Nil) FeatureEnabled(key string, defaultValue uint64) bool {
-	return defaultRandomGenerator.Random()%100 < min(defaultValue, 100)
+	return nilRandom.Uint64()%100 < min(defaultValue, 100)
 }
 
 func (n *Nil) FeatureEnabledForID(key string, id uint64, defaultPercentage uint32) bool {

snapshot/snapshot.go

@@ -10,96 +10,79 @@ import (
 	"github.com/lyft/goruntime/snapshot/entry"
 )
 
-func min(lhs uint64, rhs uint64) uint64 {
-	if lhs < rhs {
-		return lhs
-	} else {
-		return rhs
-	}
-}
-
-// Random number generator. Implementations should be thread safe.
-type RandomGenerator interface {
-	// @return uint64 a new random number.
-	Random() uint64
-}
-
-// Implementation of RandomGenerator that uses a time seeded random generator.
-type randomGeneratorImpl struct {
-	sync.Mutex
-	random *rand.Rand
-}
-
-func (r *randomGeneratorImpl) Random() uint64 {
-	r.Lock()
-	v := uint64(r.random.Int63())
-	r.Unlock()
-	return v
+type random struct {
+	mu sync.Mutex
+	rr *rand.Rand
 }
 
-var defaultRandomGenerator RandomGenerator = &randomGeneratorImpl{
-	random: rand.New(rand.NewSource(time.Now().UnixNano())),
+func (r *random) Uint64() uint64 {
+	r.mu.Lock()
+	x := r.rr.Int63()
+	r.mu.Unlock()
+	return uint64(x)
 }
 
 // Implementation of Snapshot for the filesystem loader.
 type Snapshot struct {
 	entries map[string]*entry.Entry
+	rand    random
 }
 
 func New() (s *Snapshot) {
-	s = &Snapshot{
+	rr := random{
+		rr: rand.New(rand.NewSource(time.Now().UnixNano())),
+	}
+	return &Snapshot{
 		entries: make(map[string]*entry.Entry),
+		rand:    rr,
 	}
+}
 
-	return
+func min(lhs, rhs uint64) uint64 {
+	if lhs <= rhs {
+		return lhs
+	}
+	return rhs
 }
 
 func (s *Snapshot) FeatureEnabled(key string, defaultValue uint64) bool {
-	return defaultRandomGenerator.Random()%100 < min(s.GetInteger(key, defaultValue), 100)
+	return s.rand.Uint64()%100 < min(s.GetInteger(key, defaultValue), 100)
 }
 
 // FeatureEnabledForID checks that the crc32 of the id and key's byte value falls within the mod of
 // the 0-100 value for the given feature. Use this method for "sticky" features
 func (s *Snapshot) FeatureEnabledForID(key string, id uint64, defaultPercentage uint32) bool {
-	if e, ok := s.Entries()[key]; ok {
-		if e.Uint64Valid {
-			return enabled(id, uint32(e.Uint64Value), key)
-		}
+	if e := s.entries[key]; e != nil && e.Uint64Valid {
+		return enabled(id, uint32(e.Uint64Value), key)
 	}
-
 	return enabled(id, defaultPercentage, key)
 }
 
 func (s *Snapshot) Get(key string) string {
-	e, ok := s.entries[key]
-	if ok {
+	if e := s.entries[key]; e != nil {
 		return e.StringValue
-	} else {
-		return ""
 	}
+	return ""
 }
 
 func (s *Snapshot) GetInteger(key string, defaultValue uint64) uint64 {
-	e, ok := s.entries[key]
-	if ok && e.Uint64Valid {
+	if e := s.entries[key]; e != nil && e.Uint64Valid {
 		return e.Uint64Value
-	} else {
-		return defaultValue
 	}
+	return defaultValue
 }
 
 // GetModified returns the last modified timestamp for key. If key does not
 // exist, the zero value for time.Time is returned.
 func (s *Snapshot) GetModified(key string) time.Time {
-	if e, ok := s.entries[key]; ok {
+	if e := s.entries[key]; e != nil {
 		return e.Modified
 	}
-
 	return time.Time{}
 }
 
 func (s *Snapshot) Keys() []string {
-	ret := []string{}
+	ret := make([]string, 0, len(s.entries))
 	for key := range s.entries {
 		ret = append(ret, key)
 	}
@@ -116,14 +99,11 @@ func (s *Snapshot) SetEntry(key string, e *entry.Entry) {
 
 func enabled(id uint64, percentage uint32, feature string) bool {
 	uid := crc(id, feature)
-
 	return uid%100 < percentage
 }
 
 func crc(id uint64, feature string) uint32 {
 	b := make([]byte, 8, len(feature)+8)
 	binary.LittleEndian.PutUint64(b, id)
-	b = append(b, []byte(feature)...)
-
-	return crc32.ChecksumIEEE(b)
+	return crc32.ChecksumIEEE(append(b, feature...))
 }

snapshot/snapshot_test.go

@@ -1,7 +1,9 @@
 package snapshot
 
 import (
-	"math/rand"
+	"runtime"
+	"sync"
+	"sync/atomic"
 	"testing"
 	"time"
 
@@ -10,17 +12,18 @@ import (
 )
 
 func TestRandomGeneratorImpl_Random_Race(t *testing.T) {
-	rgi := &randomGeneratorImpl{random: rand.New(rand.NewSource(time.Now().UnixNano()))}
-
-	go func() {
-		for i := 0; i < 100; i++ {
-			rgi.Random()
-		}
-	}()
-
-	for i := 0; i < 100; i++ {
-		rgi.Random()
+	snap := New()
+	var wg sync.WaitGroup
+	for i := 0; i < 64; i++ {
+		wg.Add(1)
+		go func() {
+			defer wg.Done()
+			for i := 0; i < 100; i++ {
+				snap.rand.Uint64()
+			}
+		}()
 	}
+	wg.Wait()
 }
 
 func TestSnapshot_FeatureEnabledForID(t *testing.T) {
@@ -59,3 +62,62 @@ func TestSnapshot_GetModified(t *testing.T) {
 	ss.entries["foo"] = &entry.Entry{Modified: now}
 	assert.Equal(t, now, ss.GetModified("foo"))
 }
+
+func BenchmarkCRC(b *testing.B) {
+	const a = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
+	const key = a + a
+	if len(key) != 64 {
+		panic(len(key))
+	}
+	b.SetBytes(int64(8 + len(key)))
+	for i := 0; i < b.N; i++ {
+		crc(uint64(i), key)
+	}
+}
+
+func BenchmarkEnabled(b *testing.B) {
+	const a = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
+	const key = a + a
+	if len(key) != 64 {
+		panic(len(key))
+	}
+	b.SetBytes(int64(8 + len(key)))
+	for i := 0; i < b.N; i++ {
+		enabled(uint64(1), 50, key)
+	}
+}
+
+func setupFeatureEnabled(b *testing.B, key string) *Snapshot {
+	snap := New()
+	snap.entries[key] = &entry.Entry{
+		Uint64Value: 50,
+		Uint64Valid: true,
+	}
+	b.ResetTimer()
+	return snap
+}
+
+func BenchmarkFeatureEnabled(b *testing.B) {
+	const key = "this_is_a_test_key"
+	snap := setupFeatureEnabled(b, key)
+	for i := 0; i < b.N; i++ {
+		snap.FeatureEnabled(key, 100)
+	}
+}
+
+func BenchmarkFeatureEnabled_Parallel(b *testing.B) {
+	const key = "this_is_a_test_key"
+	var snaps [4]*Snapshot
+	for i := 0; i < 4; i++ {
+		snaps[i] = setupFeatureEnabled(b, key)
+	}
+	n := new(int32)
+	runtime.GOMAXPROCS(runtime.NumCPU() * 8)
+	b.RunParallel(func(pb *testing.PB) {
+		i := int(atomic.AddInt32(n, 1))
+		snap := snaps[i%len(snaps)]
+		for pb.Next() {
+			snap.FeatureEnabled(key, 100)
+		}
+	})
+}