From 8fc9aa97eafe09f1624c837453d07aaff5eb37b6 Mon Sep 17 00:00:00 2001 From: Nathan VanBenschoten Date: Thu, 30 Dec 2021 23:09:52 -0500 Subject: [PATCH] kv: combine heap allocations in maybeStripInFlightWrites This commit combines two of the heap allocations incurred by 1PC calls to `maybeStripInFlightWrites` when making a shallow copy of the provided `BatchRequest` into a single allocation. In a write-heavy workload, these were observed to account for about **0.8%** of all heap allocations, meaning that this change should reduce heap allocations in that workload by about **0.4%**. ``` File: cockroach Type: alloc_objects Time: Dec 31, 2021 at 3:51am (UTC) Active filters: focus=maybeStripInFlightWrites Showing nodes accounting for 2259283, 1.37% of 164666499 total ----------------------------------------------------------+------------- flat flat% sum% cum cum% calls calls% + context ----------------------------------------------------------+------------- ... ----------------------------------------------------------+------------- 506152 100% | github.com/cockroachdb/cockroach/pkg/kv/kvserver.(*Replica).sendWithRangeID /go/src/github.com/cockroachdb/cockroach/pkg/kv/kvserver/replica_send.go:140 0 0% 0.63% 506152 0.31% | github.com/cockroachdb/cockroach/pkg/kv/kvserver.maybeStripInFlightWrites /go/src/github.com/cockroachdb/cockroach/pkg/kv/kvserver/replica_batch_updates.go:58 506152 100% | github.com/cockroachdb/cockroach/pkg/roachpb.(*EndTxnRequest).ShallowCopy /go/src/github.com/cockroachdb/cockroach/pkg/roachpb/api.go:797 (inline) ----------------------------------------------------------+------------- 720901 100% | github.com/cockroachdb/cockroach/pkg/kv/kvserver.(*Replica).sendWithRangeID /go/src/github.com/cockroachdb/cockroach/pkg/kv/kvserver/replica_send.go:140 0 0% 0.63% 720901 0.44% | github.com/cockroachdb/cockroach/pkg/kv/kvserver.maybeStripInFlightWrites /go/src/github.com/cockroachdb/cockroach/pkg/kv/kvserver/replica_batch_updates.go:62 720901 100% | github.com/cockroachdb/cockroach/pkg/roachpb.(*RequestUnion).MustSetInner /go/src/github.com/cockroachdb/cockroach/pkg/roachpb/batch_generated.go:385 ----------------------------------------------------------+------------- ``` --- pkg/kv/kvserver/replica_batch_updates.go | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/pkg/kv/kvserver/replica_batch_updates.go b/pkg/kv/kvserver/replica_batch_updates.go index 70269d523d21..2c1bd69904e0 100644 --- a/pkg/kv/kvserver/replica_batch_updates.go +++ b/pkg/kv/kvserver/replica_batch_updates.go @@ -55,11 +55,17 @@ func maybeStripInFlightWrites(ba *roachpb.BatchRequest) (*roachpb.BatchRequest, // append. Code below can use origET to recreate the in-flight write set if // any elements remain in it. origET := et - et = origET.ShallowCopy().(*roachpb.EndTxnRequest) + etAlloc := new(struct { + et roachpb.EndTxnRequest + union roachpb.RequestUnion_EndTxn + }) + etAlloc.et = *origET // shallow copy + etAlloc.union.EndTxn = &etAlloc.et + et = &etAlloc.et et.InFlightWrites = nil et.LockSpans = et.LockSpans[:len(et.LockSpans):len(et.LockSpans)] // immutable ba.Requests = append([]roachpb.RequestUnion(nil), ba.Requests...) - ba.Requests[len(ba.Requests)-1].MustSetInner(et) + ba.Requests[len(ba.Requests)-1].Value = &etAlloc.union // Fast-path: If we know that this batch contains all of the transaction's // in-flight writes, then we can avoid searching in the in-flight writes set