Merge cockroachdb#86515

86515: storage: improve `ComputeStats` performance r=aliher1911 a=erikgrinaker This patch omits a `sort.Search()` call during `ComputeStats()` when possible, which essentially recovers the remaining no-range-key regression since 22.1. ``` name old time/op new time/op delta MVCCComputeStats_Pebble/valueSize=32/numRangeKeys=0-24 156ms ± 1% 145ms ± 1% -7.28% (p=0.000 n=10+10) MVCCComputeStats_Pebble/valueSize=32/numRangeKeys=1-24 190ms ± 1% 178ms ± 1% -6.24% (p=0.000 n=10+10) MVCCComputeStats_Pebble/valueSize=32/numRangeKeys=100-24 210ms ± 1% 187ms ± 1% -10.89% (p=0.000 n=10+10) ``` Resolves cockroachdb#84544. Release justification: bug fixes and low-risk updates to new functionality Release note: None Co-authored-by: Erik Grinaker <[email protected]>
wenyihu6 · Aug 22, 2022 · 0e861ec · 0e861ec
2 parents a50a783 + 1eaa593
commit 0e861ec
Show file tree

Hide file tree

Showing 2 changed files with 16 additions and 10 deletions.
diff --git a/pkg/storage/mvcc.go b/pkg/storage/mvcc.go
@@ -5547,8 +5547,10 @@ func computeStatsForIterWithVisitors(
 		// key.
 		var nextRangeTombstone hlc.Timestamp
 		if isValue {
-			if v, ok := rangeTombstones.FirstAbove(unsafeKey.Timestamp); ok {
-				nextRangeTombstone = v.Timestamp
+			if !rangeTombstones.IsEmpty() && unsafeKey.Timestamp.LessEq(rangeTombstones.Newest()) {
+				if v, ok := rangeTombstones.FirstAbove(unsafeKey.Timestamp); ok {
+					nextRangeTombstone = v.Timestamp
+				}
 			}
 		}
 

diff --git a/pkg/storage/mvcc_key.go b/pkg/storage/mvcc_key.go
@@ -727,21 +727,25 @@ func (v MVCCRangeKeyVersions) FirstAbove(ts hlc.Timestamp) (MVCCRangeKeyVersion,
 	// This is kind of odd due to sort.Search() semantics: we do a binary search
 	// for the first range key that's below the timestamp, then return the
 	// previous range key if any.
-	if i := sort.Search(len(v), func(i int) bool {
-		return v[i].Timestamp.Less(ts)
-	}); i > 0 {
-		return v[i-1], true
+	if length := len(v); length > 0 {
+		if i := sort.Search(length, func(i int) bool {
+			return v[i].Timestamp.Less(ts)
+		}); i > 0 {
+			return v[i-1], true
+		}
 	}
 	return MVCCRangeKeyVersion{}, false
 }
 
 // FirstBelow does a binary search for the first range key version at or below
 // the given timestamp. Returns false if no matching range key was found.
 func (v MVCCRangeKeyVersions) FirstBelow(ts hlc.Timestamp) (MVCCRangeKeyVersion, bool) {
-	if i := sort.Search(len(v), func(i int) bool {
-		return v[i].Timestamp.LessEq(ts)
-	}); i < len(v) {
-		return v[i], true
+	if length := len(v); length > 0 {
+		if i := sort.Search(length, func(i int) bool {
+			return v[i].Timestamp.LessEq(ts)
+		}); i < length {
+			return v[i], true
+		}
 	}
 	return MVCCRangeKeyVersion{}, false
 }