From 06c1adf483fbbfcd82b28a6346b62da5b0e5ca0d Mon Sep 17 00:00:00 2001
From: Andrei Matei <andrei@cockroachlabs.com>
Date: Wed, 19 Sep 2018 21:58:59 -0400
Subject: [PATCH] client: don't allocate some commit batches

When commiting, the client.Txn used to allocate a slice of requests
(inside a BatchRequest), a RequestUnion, and an EndTransactionRequest.
This shows up on profiles of a single node read-heavy workload.
This patch moves to pre-allocating these inside a Txn. This works since
there's no concurrent commits on a single txn.

Release note: None
---
 pkg/internal/client/txn.go | 36 ++++++++++++++++++++++++++++++++++--
 1 file changed, 34 insertions(+), 2 deletions(-)

diff --git a/pkg/internal/client/txn.go b/pkg/internal/client/txn.go
index e1a3f1860023..06d0f9900dd4 100644
--- a/pkg/internal/client/txn.go
+++ b/pkg/internal/client/txn.go
@@ -38,6 +38,18 @@ type Txn struct {
 	// typ indicates the type of transaction.
 	typ TxnType
 
+	// alloc holds pre-allocated fields that can be used to avoid heap allocations
+	// for batches with lifetimes tied to a Txn.
+	// TODO(andrei): A lot more things should be pre-allocated or otherwise pooled
+	// - in particular the roachpb.Transaction proto and the TxnCoordSender which
+	// are quite large. Pooling them would also force us to clean up their
+	// lifetimes, which is a good idea on its own.
+	alloc struct {
+		requests [1]roachpb.RequestUnion
+		etUnion  roachpb.RequestUnion_EndTransaction
+		et       roachpb.EndTransactionRequest
+	}
+
 	// gatewayNodeID, if != 0, is the ID of the node on whose behalf this
 	// transaction is running. Normally this is the current node, but in the case
 	// of Txns created on remote nodes by DistSQL this will be the gateway.
@@ -509,8 +521,7 @@ func (txn *Txn) Run(ctx context.Context, b *Batch) error {
 }
 
 func (txn *Txn) commit(ctx context.Context) error {
-	var ba roachpb.BatchRequest
-	ba.Add(endTxnReq(true /* commit */, txn.deadline(), txn.systemConfigTrigger))
+	ba := txn.getCommitReq(txn.deadline(), txn.systemConfigTrigger)
 	_, pErr := txn.Send(ctx, ba)
 	if pErr == nil {
 		for _, t := range txn.commitTriggers {
@@ -534,6 +545,27 @@ func (txn *Txn) CleanupOnError(ctx context.Context, err error) {
 	}
 }
 
+func (txn *Txn) getCommitReq(deadline *hlc.Timestamp, hasTrigger bool) roachpb.BatchRequest {
+	ba := roachpb.BatchRequest{
+		Requests: txn.alloc.requests[:1],
+	}
+	etReq := &txn.alloc.et
+	etUnion := &txn.alloc.etUnion
+	ba.Requests[0].Value = etUnion
+	etUnion.EndTransaction = etReq
+	etReq.Reset()
+	etReq.Commit = true
+	etReq.Deadline = deadline
+	if hasTrigger {
+		etReq.InternalCommitTrigger = &roachpb.InternalCommitTrigger{
+			ModifiedSpanTrigger: &roachpb.ModifiedSpanTrigger{
+				SystemConfigSpan: true,
+			},
+		}
+	}
+	return ba
+}
+
 // Commit is the same as CommitOrCleanup but will not attempt to clean
 // up on failure. This can be used when the caller is prepared to do proper
 // cleanup.