From bf28d2ef305149d5b24150b3e01e2a63bb54b69c Mon Sep 17 00:00:00 2001
From: Pavel Kalinnikov <pavel@cockroachlabs.com>
Date: Fri, 3 Nov 2023 10:02:58 +0000
Subject: [PATCH] logstore: pool MVCCStats to avoid hot path allocs

Before this commit, logstore.logAppend allocated MVCCStats on heap
despite its lifetime being confined within this function's scope.

This commit switches to allocating MVCCStats on a sync.Pool. Since
logAppend already has another pooled roachpb.Value with exactly the same
lifetime, coalesce the allocation of both and put them in the same pool.
This makes the cost of this change zero.

Microbenchmark results:

```
// before
BenchmarkLogStore_StoreEntries/bytes=1.0_KiB-24 837254   1270 ns/op  333 B/op  1 allocs/op
// after
BenchmarkLogStore_StoreEntries/bytes=1.0_KiB-24 1000000  1153 ns/op  157 B/op  0 allocs/op
```

Epic: none
Release note: none
---
 pkg/kv/kvserver/logstore/logstore.go | 29 ++++++++++++++++++++--------
 1 file changed, 21 insertions(+), 8 deletions(-)

diff --git a/pkg/kv/kvserver/logstore/logstore.go b/pkg/kv/kvserver/logstore/logstore.go
index efa7f9fc7945..928bbf56ce5b 100644
--- a/pkg/kv/kvserver/logstore/logstore.go
+++ b/pkg/kv/kvserver/logstore/logstore.go
@@ -360,8 +360,13 @@ var nonBlockingSyncWaiterCallbackPool = sync.Pool{
 	New: func() interface{} { return new(nonBlockingSyncWaiterCallback) },
 }
 
-var valPool = sync.Pool{
-	New: func() interface{} { return &roachpb.Value{} },
+var logAppendPool = sync.Pool{
+	New: func() interface{} {
+		return new(struct {
+			roachpb.Value
+			enginepb.MVCCStats
+		})
+	},
 }
 
 // logAppend adds the given entries to the raft log. Takes the previous log
@@ -382,13 +387,21 @@ func logAppend(
 	if len(entries) == 0 {
 		return prev, nil
 	}
-	var diff enginepb.MVCCStats
-	opts := storage.MVCCWriteOptions{
-		Stats: &diff,
-	}
-	value := valPool.Get().(*roachpb.Value)
+
+	// NB: the Value and MVCCStats lifetime is this function, so we coalesce their
+	// allocation into the same pool.
+	// TODO(pavelkalinnikov): figure out why they escape into the heap, and find a
+	// way to avoid the pool.
+	v := logAppendPool.Get().(*struct {
+		roachpb.Value
+		enginepb.MVCCStats
+	})
+	defer logAppendPool.Put(v)
+	value, diff := &v.Value, &v.MVCCStats
 	value.RawBytes = value.RawBytes[:0]
-	defer valPool.Put(value)
+	diff.Reset()
+
+	opts := storage.MVCCWriteOptions{Stats: diff}
 	for i := range entries {
 		ent := &entries[i]
 		key := keys.RaftLogKeyFromPrefix(raftLogPrefix, kvpb.RaftIndex(ent.Index))