storage/spanlatch: use linked list instead of map for readSet

This change replaces the Manager's `readSet` map implementation with
a linked-list implementation. This provides the following speedup:

```
name                                          old time/op    new time/op    delta
LatchManagerReadOnlyMix/size=1-4                 683ns ± 9%     404ns ±10%  -40.85%  (p=0.008 n=5+5)
LatchManagerReadOnlyMix/size=4-4                 660ns ± 7%     382ns ± 5%  -42.17%  (p=0.008 n=5+5)
LatchManagerReadOnlyMix/size=16-4                684ns ±10%     367ns ± 5%  -46.27%  (p=0.008 n=5+5)
LatchManagerReadOnlyMix/size=64-4                683ns ± 8%     370ns ± 1%  -45.75%  (p=0.016 n=5+4)
LatchManagerReadOnlyMix/size=128-4               678ns ± 4%     398ns ±14%  -41.27%  (p=0.008 n=5+5)
LatchManagerReadOnlyMix/size=256-4               652ns ± 4%     385ns ± 4%  -40.95%  (p=0.008 n=5+5)
LatchManagerReadWriteMix/readsPerWrite=0-4       594ns ±16%     629ns ±17%     ~     (p=0.222 n=5+5)
LatchManagerReadWriteMix/readsPerWrite=1-4       603ns ± 1%     552ns ± 7%   -8.39%  (p=0.008 n=5+5)
LatchManagerReadWriteMix/readsPerWrite=4-4       621ns ± 4%     576ns ± 5%   -7.28%  (p=0.008 n=5+5)
LatchManagerReadWriteMix/readsPerWrite=16-4      649ns ± 2%     541ns ±13%  -16.69%  (p=0.008 n=5+5)
LatchManagerReadWriteMix/readsPerWrite=64-4      474ns ± 5%     423ns ±29%     ~     (p=0.151 n=5+5)
LatchManagerReadWriteMix/readsPerWrite=128-4     413ns ± 2%     362ns ±16%     ~     (p=0.095 n=5+5)
LatchManagerReadWriteMix/readsPerWrite=256-4     448ns ±14%     314ns ±13%  -29.85%  (p=0.008 n=5+5)

name                                          old alloc/op   new alloc/op   delta
LatchManagerReadOnlyMix/size=1-4                  191B ± 0%      160B ± 0%  -16.23%  (p=0.008 n=5+5)
LatchManagerReadOnlyMix/size=4-4                  191B ± 0%      160B ± 0%  -16.23%  (p=0.008 n=5+5)
LatchManagerReadOnlyMix/size=16-4                 191B ± 0%      160B ± 0%  -16.23%  (p=0.008 n=5+5)
LatchManagerReadOnlyMix/size=64-4                 191B ± 0%      160B ± 0%     ~     (p=0.079 n=4+5)
LatchManagerReadOnlyMix/size=128-4                191B ± 0%      160B ± 0%  -16.23%  (p=0.008 n=5+5)
LatchManagerReadOnlyMix/size=256-4                191B ± 0%      160B ± 0%  -16.23%  (p=0.008 n=5+5)
LatchManagerReadWriteMix/readsPerWrite=0-4        144B ± 0%      160B ± 0%  +11.11%  (p=0.008 n=5+5)
LatchManagerReadWriteMix/readsPerWrite=1-4        144B ± 0%      160B ± 0%  +11.11%  (p=0.008 n=5+5)
LatchManagerReadWriteMix/readsPerWrite=4-4        144B ± 0%      160B ± 0%  +11.11%  (p=0.008 n=5+5)
LatchManagerReadWriteMix/readsPerWrite=16-4       144B ± 0%      160B ± 0%  +11.11%  (p=0.008 n=5+5)
LatchManagerReadWriteMix/readsPerWrite=64-4       144B ± 0%      160B ± 0%  +11.11%  (p=0.008 n=5+5)
LatchManagerReadWriteMix/readsPerWrite=128-4      144B ± 0%      160B ± 0%  +11.11%  (p=0.008 n=5+5)
LatchManagerReadWriteMix/readsPerWrite=256-4      144B ± 0%      160B ± 0%  +11.11%  (p=0.008 n=5+5)

name                                          old allocs/op  new allocs/op  delta
LatchManagerReadOnlyMix/size=1-4                  1.00 ± 0%      1.00 ± 0%     ~     (all equal)
LatchManagerReadOnlyMix/size=4-4                  1.00 ± 0%      1.00 ± 0%     ~     (all equal)
LatchManagerReadOnlyMix/size=16-4                 1.00 ± 0%      1.00 ± 0%     ~     (all equal)
LatchManagerReadOnlyMix/size=64-4                 1.00 ± 0%      1.00 ± 0%     ~     (all equal)
LatchManagerReadOnlyMix/size=128-4                1.00 ± 0%      1.00 ± 0%     ~     (all equal)
LatchManagerReadOnlyMix/size=256-4                1.00 ± 0%      1.00 ± 0%     ~     (all equal)
LatchManagerReadWriteMix/readsPerWrite=0-4        1.00 ± 0%      1.00 ± 0%     ~     (all equal)
LatchManagerReadWriteMix/readsPerWrite=1-4        1.00 ± 0%      1.00 ± 0%     ~     (all equal)
LatchManagerReadWriteMix/readsPerWrite=4-4        1.00 ± 0%      1.00 ± 0%     ~     (all equal)
LatchManagerReadWriteMix/readsPerWrite=16-4       1.00 ± 0%      1.00 ± 0%     ~     (all equal)
LatchManagerReadWriteMix/readsPerWrite=64-4       1.00 ± 0%      1.00 ± 0%     ~     (all equal)
LatchManagerReadWriteMix/readsPerWrite=128-4      1.00 ± 0%      1.00 ± 0%     ~     (all equal)
LatchManagerReadWriteMix/readsPerWrite=256-4      1.00 ± 0%      1.00 ± 0%     ~     (all equal)
```

The change also makes Manager's zero value completely usable.

Release note: None

pkg/storage/spanlatch/interval_btree.go

@@ -53,9 +53,9 @@ func cmp(a, b *latch) int {
 	if c != 0 {
 		return c
 	}
-	if a.id() < b.id() {
+	if a.id < b.id {
 		return -1
-	} else if a.id() > b.id() {
+	} else if a.id > b.id {
 		return 1
 	} else {
 		return 0

pkg/storage/spanlatch/interval_btree_test.go

@@ -593,8 +593,8 @@ func TestBTreeCmp(t *testing.T) {
 	for _, tc := range testCases {
 		name := fmt.Sprintf("cmp(%s:%d,%s:%d)", tc.spanA, tc.idA, tc.spanB, tc.idB)
 		t.Run(name, func(t *testing.T) {
-			laA := &latch{meta: tc.idA, span: tc.spanA}
-			laB := &latch{meta: tc.idB, span: tc.spanB}
+			laA := &latch{id: tc.idA, span: tc.spanA}
+			laB := &latch{id: tc.idB, span: tc.spanB}
 			require.Equal(t, tc.exp, cmp(laA, laB))
 		})
 	}

pkg/storage/spanlatch/list.go

@@ -0,0 +1,54 @@
+// Copyright 2018 The Cockroach Authors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
+// implied. See the License for the specific language governing
+// permissions and limitations under the License.
+
+package spanlatch
+
+// latchList is a double-linked circular list of *latch elements.
+type latchList struct {
+	root latch
+	len  int
+}
+
+func (ll *latchList) front() *latch {
+	if ll.len == 0 {
+		return nil
+	}
+	return ll.root.next
+}
+
+func (ll *latchList) lazyInit() {
+	if ll.root.next == nil {
+		ll.root.next = &ll.root
+		ll.root.prev = &ll.root
+	}
+}
+
+func (ll *latchList) pushBack(la *latch) {
+	ll.lazyInit()
+	at := ll.root.prev
+	n := at.next
+	at.next = la
+	la.prev = at
+	la.next = n
+	n.prev = la
+	ll.len++
+}
+
+func (ll *latchList) remove(la *latch) {
+	la.prev.next = la.next
+	la.next.prev = la.prev
+	la.next = nil // avoid memory leaks
+	la.prev = nil // avoid memory leaks
+	ll.len--
+}

pkg/storage/spanlatch/manager.go

@@ -50,6 +50,8 @@ import (
 // performed under lock is linear with respect to the number of spans that a
 // latch acquisition declares but NOT linear with respect to the number of other
 // latch attempts that it will wait on.
+//
+// Manager's zero value can be used directly.
 type Manager struct {
 	mu      syncutil.Mutex
 	idAlloc uint64
@@ -59,48 +61,22 @@ type Manager struct {
 // scopedManager is a latch manager scoped to either local or global keys.
 // See spanset.SpanScope.
 type scopedManager struct {
-	rSet  map[*latch]struct{}
-	trees [spanset.NumSpanAccess]btree
-}
-
-// New creates a new Manager.
-func New() *Manager {
-	m := new(Manager)
-	for s := spanset.SpanScope(0); s < spanset.NumSpanScope; s++ {
-		m.scopes[s] = scopedManager{
-			rSet: make(map[*latch]struct{}),
-		}
-	}
-	return m
+	readSet latchList
+	trees   [spanset.NumSpanAccess]btree
 }
 
 // latches are stored in the Manager's btrees. They represent the latching
 // of a single key span.
 type latch struct {
-	meta uint64 // high bit: inRSet; lower 63 bits: id
-	span roachpb.Span
-	ts   hlc.Timestamp
-	done *signal
-}
-
-func (la *latch) inRSet() bool {
-	return la.meta>>63 == 1
+	id         uint64
+	span       roachpb.Span
+	ts         hlc.Timestamp
+	done       *signal
+	next, prev *latch // readSet linked-list.
 }
 
-func (la *latch) setInRSet(b bool) {
-	if b {
-		la.meta |= (1 << 63)
-	} else {
-		la.meta &^= (1 << 63)
-	}
-}
-
-func (la *latch) id() uint64 {
-	return la.meta &^ (1 << 63)
-}
-
-func (la *latch) setID(id uint64) {
-	la.meta = id &^ (1 << 63)
+func (la *latch) inReadSet() bool {
+	return la.next != nil
 }
 
 // Guard is a handle to a set of acquired latches. It is returned by
@@ -255,9 +231,7 @@ func (m *Manager) snapshotLocked(spans *spanset.SpanSet) snapshot {
 		writing := len(spans.GetSpans(spanset.SpanReadWrite, s)) > 0
 
 		if writing {
-			if len(sm.rSet) > 0 {
-				sm.flushReadSetLocked()
-			}
+			sm.flushReadSetLocked()
 			snap.trees[s][spanset.SpanReadOnly] = sm.trees[spanset.SpanReadOnly].Clone()
 		}
 		if writing || reading {
@@ -269,19 +243,11 @@ func (m *Manager) snapshotLocked(spans *spanset.SpanSet) snapshot {
 
 // flushReadSetLocked flushes the read set into the read interval tree.
 func (sm *scopedManager) flushReadSetLocked() {
-	for latch := range sm.rSet {
-		latch.setInRSet(false)
+	for sm.readSet.len > 0 {
+		latch := sm.readSet.front()
+		sm.readSet.remove(latch)
 		sm.trees[spanset.SpanReadOnly].Set(latch)
 	}
-	if realloc := len(sm.rSet) > 16; realloc {
-		// TODO(nvanbenschoten): never re-alloc in go1.11.
-		sm.rSet = make(map[*latch]struct{})
-	} else {
-		// NB: hitting map-clearing range fast-path.
-		for latch := range sm.rSet {
-			delete(sm.rSet, latch)
-		}
-	}
 }
 
 // insertLocked inserts the latches owned by the provided Guard into the
@@ -293,13 +259,12 @@ func (m *Manager) insertLocked(lg *Guard) {
 			latches := lg.latches(s, a)
 			for i := range latches {
 				latch := &latches[i]
-				latch.setID(m.nextID())
+				latch.id = m.nextID()
 				switch a {
 				case spanset.SpanReadOnly:
 					// Add reads to the rSet. They only need to enter the read
 					// tree if they're flushed by a write capturing a snapshot.
-					latch.setInRSet(true)
-					sm.rSet[latch] = struct{}{}
+					sm.readSet.pushBack(latch)
 				case spanset.SpanReadWrite:
 					// Add writes directly to the write tree.
 					sm.trees[spanset.SpanReadWrite].Set(latch)
@@ -424,8 +389,8 @@ func (m *Manager) removeLocked(lg *Guard) {
 			latches := lg.latches(s, a)
 			for i := range latches {
 				latch := &latches[i]
-				if latch.inRSet() {
-					delete(sm.rSet, latch)
+				if latch.inReadSet() {
+					sm.readSet.remove(latch)
 				} else {
 					sm.trees[a].Delete(latch)
 				}

pkg/storage/spanlatch/manager_test.go

@@ -117,7 +117,7 @@ func (m *Manager) MustAcquireChCtx(
 
 func TestLatchManager(t *testing.T) {
 	defer leaktest.AfterTest(t)()
-	m := New()
+	var m Manager
 
 	// Try latch with no overlapping already-acquired lathes.
 	lg1 := m.MustAcquire(spans("a", "", write), zeroTS)
@@ -140,7 +140,7 @@ func TestLatchManager(t *testing.T) {
 
 func TestLatchManagerNoWaitOnReadOnly(t *testing.T) {
 	defer leaktest.AfterTest(t)()
-	m := New()
+	var m Manager
 
 	// Acquire latch for read-only span.
 	m.MustAcquire(spans("a", "", read), zeroTS)
@@ -151,7 +151,7 @@ func TestLatchManagerNoWaitOnReadOnly(t *testing.T) {
 
 func TestLatchManagerWriteWaitForMultipleReads(t *testing.T) {
 	defer leaktest.AfterTest(t)()
-	m := New()
+	var m Manager
 
 	// Acquire latch for read-only span.
 	lg1 := m.MustAcquire(spans("a", "", read), zeroTS)
@@ -179,7 +179,7 @@ func TestLatchManagerWriteWaitForMultipleReads(t *testing.T) {
 
 func TestLatchManagerMultipleOverlappingLatches(t *testing.T) {
 	defer leaktest.AfterTest(t)()
-	m := New()
+	var m Manager
 
 	// Acquire multiple latches.
 	lg1C := m.MustAcquireCh(spans("a", "", write), zeroTS)
@@ -199,7 +199,7 @@ func TestLatchManagerMultipleOverlappingLatches(t *testing.T) {
 
 func TestLatchManagerMultipleOverlappingSpans(t *testing.T) {
 	defer leaktest.AfterTest(t)()
-	m := New()
+	var m Manager
 
 	// Acquire multiple latches.
 	lg1 := m.MustAcquire(spans("a", "", write), zeroTS)
@@ -423,7 +423,7 @@ func TestLatchManagerDependentLatches(t *testing.T) {
 					c.ts1, c.ts2 = c.ts2, c.ts1
 				}
 
-				m := New()
+				var m Manager
 				lg1 := m.MustAcquire(c.sp1, c.ts1)
 				lg2C := m.MustAcquireCh(c.sp2, c.ts2)
 				if c.dependent {
@@ -443,7 +443,7 @@ func TestLatchManagerDependentLatches(t *testing.T) {
 
 func TestLatchManagerContextCancellation(t *testing.T) {
 	defer leaktest.AfterTest(t)()
-	m := New()
+	var m Manager
 
 	// Attempt to acquire three latches that all block on each other.
 	lg1 := m.MustAcquire(spans("a", "", write), zeroTS)
@@ -471,7 +471,7 @@ func TestLatchManagerContextCancellation(t *testing.T) {
 func BenchmarkLatchManagerReadOnlyMix(b *testing.B) {
 	for _, size := range []int{1, 4, 16, 64, 128, 256} {
 		b.Run(fmt.Sprintf("size=%d", size), func(b *testing.B) {
-			m := New()
+			var m Manager
 			ss := spans("a", "b", read)
 			for i := 0; i < size; i++ {
 				_ = m.MustAcquire(ss, zeroTS)
@@ -488,7 +488,7 @@ func BenchmarkLatchManagerReadOnlyMix(b *testing.B) {
 func BenchmarkLatchManagerReadWriteMix(b *testing.B) {
 	for _, readsPerWrite := range []int{0, 1, 4, 16, 64, 128, 256} {
 		b.Run(fmt.Sprintf("readsPerWrite=%d", readsPerWrite), func(b *testing.B) {
-			m := New()
+			var m Manager
 			lgBuf := make(chan *Guard, 16)
 
 			spans := make([]spanset.SpanSet, b.N)