Merge #39017

39017: storage: avoid two heap allocations per KV batch r=ajwerner a=nvanbenschoten

This PR contains two different commits that each avoid a heap allocation per KV batch.

```
name                        old time/op    new time/op    delta
KV/Insert/Native/rows=1-16    96.2µs ± 1%    94.8µs ± 1%  -1.41%  (p=0.000 n=9+10)
KV/Update/Native/rows=1-16     127µs ± 2%     127µs ± 2%    ~     (p=0.959 n=8+8)
KV/Delete/Native/rows=1-16    95.4µs ± 2%    94.4µs ± 2%    ~     (p=0.234 n=8+8)
KV/Scan/Native/rows=1-16      31.4µs ± 2%    32.1µs ±23%    ~     (p=0.113 n=9+9)
KV/Insert/SQL/rows=1-16        268µs ± 5%     266µs ± 2%    ~     (p=0.549 n=9+10)
KV/Update/SQL/rows=1-16        335µs ± 2%     335µs ± 3%    ~     (p=0.631 n=10+10)
KV/Delete/SQL/rows=1-16        359µs ± 2%     358µs ± 2%    ~     (p=0.842 n=10+9)
KV/Scan/SQL/rows=1-16          194µs ± 5%     182µs ± 5%  -5.75%  (p=0.001 n=10+10)

name                        old alloc/op   new alloc/op   delta
KV/Insert/Native/rows=1-16    11.4kB ± 0%    11.4kB ± 0%    ~     (p=0.090 n=9+10)
KV/Update/Native/rows=1-16    17.3kB ± 0%    17.2kB ± 0%  -0.20%  (p=0.001 n=10+10)
KV/Delete/Native/rows=1-16    11.3kB ± 0%    11.3kB ± 0%    ~     (p=0.059 n=9+9)
KV/Scan/Native/rows=1-16      8.16kB ± 0%    8.13kB ± 0%  -0.28%  (p=0.000 n=9+9)
KV/Insert/SQL/rows=1-16       34.1kB ± 0%    34.1kB ± 0%    ~     (p=0.143 n=10+10)
KV/Update/SQL/rows=1-16       48.8kB ± 0%    48.7kB ± 0%    ~     (p=0.108 n=10+9)
KV/Delete/SQL/rows=1-16       42.2kB ± 1%    42.1kB ± 1%    ~     (p=0.165 n=10+10)
KV/Scan/SQL/rows=1-16         28.7kB ± 0%    28.6kB ± 0%  -0.22%  (p=0.000 n=10+9)

name                        old allocs/op  new allocs/op  delta
KV/Insert/Native/rows=1-16       122 ± 1%       121 ± 0%  -1.06%  (p=0.000 n=10+9)
KV/Update/Native/rows=1-16       179 ± 0%       176 ± 0%  -1.68%  (p=0.000 n=10+10)
KV/Delete/Native/rows=1-16       121 ± 0%       120 ± 0%  -0.83%  (p=0.000 n=8+8)
KV/Scan/Native/rows=1-16        79.0 ± 0%      77.0 ± 0%  -2.53%  (p=0.000 n=10+10)
KV/Insert/SQL/rows=1-16          313 ± 0%       312 ± 0%  -0.32%  (p=0.000 n=9+9)
KV/Update/SQL/rows=1-16          465 ± 0%       462 ± 0%  -0.60%  (p=0.000 n=10+8)
KV/Delete/SQL/rows=1-16          398 ± 0%       395 ± 0%  -0.93%  (p=0.000 n=10+10)
KV/Scan/SQL/rows=1-16            289 ± 0%       287 ± 0%  -0.69%  (p=0.000 n=10+8)
```

Co-authored-by: Nathan VanBenschoten <[email protected]>

pkg/server/node.go

@@ -898,14 +898,12 @@ func (n *Node) batchInternal(
 	}
 
 	var br *roachpb.BatchResponse
-
 	if err := n.stopper.RunTaskWithErr(ctx, "node.Node: batch", func(ctx context.Context) error {
 		var finishSpan func(*roachpb.BatchResponse)
 		// Shadow ctx from the outer function. Written like this to pass the linter.
 		ctx, finishSpan = n.setupSpanForIncomingRPC(ctx, grpcutil.IsLocalRequestContext(ctx))
-		defer func(br **roachpb.BatchResponse) {
-			finishSpan(*br)
-		}(&br)
+		// NB: wrapped to delay br evaluation to its value when returning.
+		defer func() { finishSpan(br) }()
 		if log.HasSpanOrEvent(ctx) {
 			log.Event(ctx, args.Summary())
 		}

pkg/sql/sem/tree/datum.go

@@ -1397,7 +1397,7 @@ func (d *DUuid) Compare(ctx *EvalContext, other Datum) int {
 	return bytes.Compare(d.GetBytes(), v.GetBytes())
 }
 
-func (d DUuid) equal(other *DUuid) bool {
+func (d *DUuid) equal(other *DUuid) bool {
 	return bytes.Equal(d.GetBytes(), other.GetBytes())
 }
 

pkg/storage/engine/rocksdb.go

@@ -2610,7 +2610,7 @@ func goToCTimestamp(ts hlc.Timestamp) C.DBTimestamp {
 func goToCTxn(txn *roachpb.Transaction) C.DBTxn {
 	var r C.DBTxn
 	if txn != nil {
-		r.id = goToCSlice(txn.ID.GetBytes())
+		r.id = goToCSlice(txn.ID.GetBytesMut())
 		r.epoch = C.uint32_t(txn.Epoch)
 		r.sequence = C.int32_t(txn.Sequence)
 		r.max_timestamp = goToCTimestamp(txn.MaxTimestamp)

pkg/util/uuid/uuid.go

@@ -117,11 +117,20 @@ func (u UUID) Variant() byte {
 	}
 }
 
-// Bytes returns a byte slice representation of the UUID.
+// bytes returns a byte slice representation of the UUID. It incurs an
+// allocation if the return value escapes.
 func (u UUID) bytes() []byte {
 	return u[:]
 }
 
+// bytesMut returns a mutable byte slice representation of the UUID. Unlike
+// bytes, it does not necessarily incur an allocation if the return value
+// escapes. Instead, the return value escaping will cause the method's receiver
+// (and any struct that it is a part of) to escape.
+func (u *UUID) bytesMut() []byte {
+	return u[:]
+}
+
 // String returns a canonical RFC-4122 string representation of the UUID:
 // xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx.
 func (u UUID) String() string {

pkg/util/uuid/uuid_wrapper.go

@@ -43,11 +43,21 @@ func (u UUID) Equal(t UUID) bool {
 	return u == t
 }
 
-// GetBytes returns the UUID as a byte slice.
+// GetBytes returns the UUID as a byte slice. It incurs an allocation if
+// the return value escapes.
 func (u UUID) GetBytes() []byte {
 	return u.bytes()
 }
 
+// GetBytesMut returns the UUID as a mutable byte slice. Unlike GetBytes,
+// it does not necessarily incur an allocation if the return value escapes.
+// Instead, the return value escaping will cause the method's receiver (and
+// any struct that it is a part of) to escape. Use only if GetBytes is causing
+// an allocation and the UUID is already on the heap.
+func (u *UUID) GetBytesMut() []byte {
+	return u.bytesMut()
+}
+
 // ToUint128 returns the UUID as a Uint128.
 func (u UUID) ToUint128() uint128.Uint128 {
 	return uint128.FromBytes(u.bytes())