Commit c713842d authored by craig[bot]'s avatar craig[bot]

Merge #50791 #50819

50791: sql: don't allocate clientConnLock in LockCommunication r=nvanbenschoten a=nvanbenschoten

This allocation was showing up as **0.28%** of a CPU profile while running TPC-E. It's easy to avoid, so do so.

<img width="1679" alt="Screen Shot 2020-06-29 at 7 42 30 PM" src="https://user-images.githubusercontent.com/5438456/86068762-30f41100-ba46-11ea-864c-76d1b271992c.png">

While here, do the same thing for `noopClientLock`.

50819: kv/kvclient: avoid letting BatchRequests escape to heap r=nvanbenschoten a=nvanbenschoten

The first one in `divideAndSendBatchToRanges` was caused by #50633.

While here, avoid the allocation in `divideAndSendParallelCommit`.
Co-authored-by: default avatarNathan VanBenschoten <[email protected]>
......@@ -656,7 +656,7 @@ func unsetCanForwardReadTimestampFlag(ctx context.Context, ba *roachpb.BatchRequ
// Assert this for our own sanity.
if _, ok := ba.GetArg(roachpb.EndTxn); ok {
log.Fatalf(ctx, "batch unexpected contained requests "+
"that need to refresh and an EndTxn request: %s", ba)
"that need to refresh and an EndTxn request: %s", ba.String())
}
return
}
......@@ -866,6 +866,7 @@ func (ds *DistSender) divideAndSendParallelCommit(
}
qiBatchIdx := batchIdx + 1
qiResponseCh := make(chan response, 1)
qiBaCopy := qiBa // avoids escape to heap
runTask := ds.rpcContext.Stopper.RunAsyncTask
if ds.disableParallelBatches {
......@@ -941,7 +942,7 @@ func (ds *DistSender) divideAndSendParallelCommit(
}
// Populate the pre-commit QueryIntent batch response. If we made it
// here then we know we can ignore intent missing errors.
qiReply.reply = qiBa.CreateReply()
qiReply.reply = qiBaCopy.CreateReply()
for _, ru := range qiReply.reply.Responses {
ru.GetQueryIntent().FoundIntent = true
}
......
......@@ -577,9 +577,7 @@ func (icc *internalClientComm) CreateSyncResult(pos CmdPos) SyncResult {
// LockCommunication is part of the ClientComm interface.
func (icc *internalClientComm) LockCommunication() ClientLock {
return &noopClientLock{
clientComm: icc,
}
return (*noopClientLock)(icc)
}
// Flush is part of the ClientComm interface.
......@@ -624,26 +622,24 @@ func (icc *internalClientComm) CreateDrainResult(pos CmdPos) DrainResult {
// noopClientLock is an implementation of ClientLock that says that no results
// have been communicated to the client.
type noopClientLock struct {
clientComm *internalClientComm
}
type noopClientLock internalClientComm
// Close is part of the ClientLock interface.
func (ncl *noopClientLock) Close() {}
// ClientPos is part of the ClientLock interface.
func (ncl *noopClientLock) ClientPos() CmdPos {
return ncl.clientComm.lastDelivered
return ncl.lastDelivered
}
// RTrim is part of the ClientLock interface.
func (ncl *noopClientLock) RTrim(_ context.Context, pos CmdPos) {
var i int
var r resWithPos
for i, r = range ncl.clientComm.results {
for i, r = range ncl.results {
if r.pos >= pos {
break
}
}
ncl.clientComm.results = ncl.clientComm.results[:i]
ncl.results = ncl.results[:i]
}
......@@ -1344,15 +1344,13 @@ func (c *conn) maybeFlush(pos sql.CmdPos) (bool, error) {
// nothing to "lock" - communication is naturally blocked as the command
// processor won't write any more results.
func (c *conn) LockCommunication() sql.ClientLock {
return &clientConnLock{flushInfo: &c.writerState.fi}
return (*clientConnLock)(&c.writerState.fi)
}
// clientConnLock is the connection's implementation of sql.ClientLock. It lets
// the sql module lock the flushing of results and find out what has already
// been flushed.
type clientConnLock struct {
*flushInfo
}
type clientConnLock flushInfo
var _ sql.ClientLock = &clientConnLock{}
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment