Merge #74336 #74338 #74339 #74340 #74344 #74346 #74347 #74350 #74353 #…

…74356 #74358 74336: kv: combine heap allocations for EndTxn requests r=tbg a=nvanbenschoten This commit combines the 3 heap allocations incurred by a call to `kv.Txn.Commit` or `kv.Txn.Rollback` when constructing the request into a single allocation. ``` name old time/op new time/op delta KV/Scan/SQL/rows=1-10 95.3µs ± 9% 93.5µs ± 6% ~ (p=0.497 n=10+9) name old alloc/op new alloc/op delta KV/Scan/SQL/rows=1-10 20.1kB ± 0% 20.1kB ± 0% ~ (p=0.345 n=10+9) name old allocs/op new allocs/op delta KV/Scan/SQL/rows=1-10 245 ± 0% 243 ± 0% -0.82% (p=0.000 n=10+9) ``` ---- This is part of a collection of assorted micro-optimizations: - #74336 - #74337 - #74338 - #74339 - #74340 - #74341 - #74342 - #74343 - #74344 - #74345 - #74346 - #74347 - #74348 Combined, these changes have the following effect on end-to-end SQL query performance: ``` name old time/op new time/op delta KV/Scan/SQL/rows=1-10 94.4µs ±10% 92.3µs ±11% -2.20% (p=0.000 n=93+93) KV/Scan/SQL/rows=10-10 102µs ±10% 99µs ±10% -2.16% (p=0.000 n=94+94) KV/Update/SQL/rows=10-10 378µs ±15% 370µs ±11% -2.04% (p=0.003 n=95+91) KV/Insert/SQL/rows=1-10 133µs ±14% 132µs ±12% ~ (p=0.738 n=95+93) KV/Insert/SQL/rows=10-10 197µs ±14% 196µs ±13% ~ (p=0.902 n=95+94) KV/Update/SQL/rows=1-10 186µs ±14% 185µs ±14% ~ (p=0.351 n=94+93) KV/Delete/SQL/rows=1-10 132µs ±13% 132µs ±14% ~ (p=0.473 n=94+94) KV/Delete/SQL/rows=10-10 254µs ±16% 250µs ±16% ~ (p=0.086 n=100+99) name old alloc/op new alloc/op delta KV/Scan/SQL/rows=1-10 20.1kB ± 0% 19.1kB ± 1% -4.91% (p=0.000 n=96+96) KV/Scan/SQL/rows=10-10 21.7kB ± 0% 20.7kB ± 1% -4.61% (p=0.000 n=96+97) KV/Delete/SQL/rows=10-10 64.0kB ± 3% 63.7kB ± 3% -0.55% (p=0.000 n=100+100) KV/Update/SQL/rows=1-10 45.8kB ± 1% 45.5kB ± 1% -0.55% (p=0.000 n=97+98) KV/Update/SQL/rows=10-10 105kB ± 1% 105kB ± 1% -0.10% (p=0.008 n=97+98) KV/Delete/SQL/rows=1-10 40.8kB ± 0% 40.7kB ± 0% -0.08% (p=0.001 n=95+96) KV/Insert/SQL/rows=1-10 37.4kB ± 1% 37.4kB ± 0% ~ (p=0.698 n=97+96) KV/Insert/SQL/rows=10-10 76.4kB ± 1% 76.4kB ± 0% ~ (p=0.822 n=99+98) name old allocs/op new allocs/op delta KV/Scan/SQL/rows=1-10 245 ± 0% 217 ± 0% -11.43% (p=0.000 n=95+92) KV/Scan/SQL/rows=10-10 280 ± 0% 252 ± 0% -10.11% (p=0.000 n=75+97) KV/Delete/SQL/rows=10-10 478 ± 0% 459 ± 0% -4.04% (p=0.000 n=94+97) KV/Delete/SQL/rows=1-10 297 ± 1% 287 ± 1% -3.34% (p=0.000 n=97+97) KV/Update/SQL/rows=1-10 459 ± 0% 444 ± 0% -3.27% (p=0.000 n=97+97) KV/Insert/SQL/rows=1-10 291 ± 0% 286 ± 0% -1.72% (p=0.000 n=82+86) KV/Update/SQL/rows=10-10 763 ± 1% 750 ± 1% -1.68% (p=0.000 n=96+98) KV/Insert/SQL/rows=10-10 489 ± 0% 484 ± 0% -1.03% (p=0.000 n=98+98) ``` 74338: sql: avoid string formatting in reportSessionDataChanges when not necessary r=otan a=nvanbenschoten This commit updates each of the `bufferableParamStatusUpdates` `sessionVar`s to have an `Equal` function, which returns whether the value of the variable is equal between two `SessionData` references. This allows `connExecutor.reportSessionDataChanges` to compare the values of each of the variables without resorting to string formatting. Instead, the variables are only string formatted if they have actually changed and need to be reported to the client. Micro-benchmarks have revealed that the string formatting for variables like "DateStyle" is a meaningful expense to incur on each transaction, especially for small OLTP transactions. ``` name old time/op new time/op delta KV/Scan/SQL/rows=1-10 94.7µs ± 8% 92.9µs ± 2% ~ (p=0.762 n=10+8) name old alloc/op new alloc/op delta KV/Scan/SQL/rows=1-10 20.1kB ± 0% 19.9kB ± 0% -1.00% (p=0.000 n=10+10) name old allocs/op new allocs/op delta KV/Scan/SQL/rows=1-10 245 ± 0% 235 ± 0% -4.08% (p=0.000 n=10+9) ``` ---- This is part of a collection of assorted micro-optimizations: - #74336 - #74337 - #74338 - #74339 - #74340 - #74341 - #74342 - #74343 - #74344 - #74345 - #74346 - #74347 - #74348 Combined, these changes have the following effect on end-to-end SQL query performance: ``` name old time/op new time/op delta KV/Scan/SQL/rows=1-10 94.4µs ±10% 92.3µs ±11% -2.20% (p=0.000 n=93+93) KV/Scan/SQL/rows=10-10 102µs ±10% 99µs ±10% -2.16% (p=0.000 n=94+94) KV/Update/SQL/rows=10-10 378µs ±15% 370µs ±11% -2.04% (p=0.003 n=95+91) KV/Insert/SQL/rows=1-10 133µs ±14% 132µs ±12% ~ (p=0.738 n=95+93) KV/Insert/SQL/rows=10-10 197µs ±14% 196µs ±13% ~ (p=0.902 n=95+94) KV/Update/SQL/rows=1-10 186µs ±14% 185µs ±14% ~ (p=0.351 n=94+93) KV/Delete/SQL/rows=1-10 132µs ±13% 132µs ±14% ~ (p=0.473 n=94+94) KV/Delete/SQL/rows=10-10 254µs ±16% 250µs ±16% ~ (p=0.086 n=100+99) name old alloc/op new alloc/op delta KV/Scan/SQL/rows=1-10 20.1kB ± 0% 19.1kB ± 1% -4.91% (p=0.000 n=96+96) KV/Scan/SQL/rows=10-10 21.7kB ± 0% 20.7kB ± 1% -4.61% (p=0.000 n=96+97) KV/Delete/SQL/rows=10-10 64.0kB ± 3% 63.7kB ± 3% -0.55% (p=0.000 n=100+100) KV/Update/SQL/rows=1-10 45.8kB ± 1% 45.5kB ± 1% -0.55% (p=0.000 n=97+98) KV/Update/SQL/rows=10-10 105kB ± 1% 105kB ± 1% -0.10% (p=0.008 n=97+98) KV/Delete/SQL/rows=1-10 40.8kB ± 0% 40.7kB ± 0% -0.08% (p=0.001 n=95+96) KV/Insert/SQL/rows=1-10 37.4kB ± 1% 37.4kB ± 0% ~ (p=0.698 n=97+96) KV/Insert/SQL/rows=10-10 76.4kB ± 1% 76.4kB ± 0% ~ (p=0.822 n=99+98) name old allocs/op new allocs/op delta KV/Scan/SQL/rows=1-10 245 ± 0% 217 ± 0% -11.43% (p=0.000 n=95+92) KV/Scan/SQL/rows=10-10 280 ± 0% 252 ± 0% -10.11% (p=0.000 n=75+97) KV/Delete/SQL/rows=10-10 478 ± 0% 459 ± 0% -4.04% (p=0.000 n=94+97) KV/Delete/SQL/rows=1-10 297 ± 1% 287 ± 1% -3.34% (p=0.000 n=97+97) KV/Update/SQL/rows=1-10 459 ± 0% 444 ± 0% -3.27% (p=0.000 n=97+97) KV/Insert/SQL/rows=1-10 291 ± 0% 286 ± 0% -1.72% (p=0.000 n=82+86) KV/Update/SQL/rows=10-10 763 ± 1% 750 ± 1% -1.68% (p=0.000 n=96+98) KV/Insert/SQL/rows=10-10 489 ± 0% 484 ± 0% -1.03% (p=0.000 n=98+98) ``` 74339: sql: avoid net.Error allocation on each readTimeoutConn.Read call r=knz a=nvanbenschoten This commit updates `readTimeoutConn.Read` to only call `errors.As` if the error from the Read of the wrapped `net.Conn` was non-nil. This avoids a heap allocation of a `net.Error` on each call to `readTimeoutConn.Read`, which was originally introduced in a8ae1bf. ``` name old time/op new time/op delta KV/Scan/SQL/rows=1-10 93.9µs ± 6% 94.6µs ± 7% ~ (p=0.631 n=10+10) name old alloc/op new alloc/op delta KV/Scan/SQL/rows=1-10 20.1kB ± 0% 20.1kB ± 0% ~ (p=0.197 n=10+10) name old allocs/op new allocs/op delta KV/Scan/SQL/rows=1-10 245 ± 0% 244 ± 0% -0.41% (p=0.000 n=10+10) ``` ---- This is part of a collection of assorted micro-optimizations: - #74336 - #74337 - #74338 - #74339 - #74340 - #74341 - #74342 - #74343 - #74344 - #74345 - #74346 - #74347 - #74348 Combined, these changes have the following effect on end-to-end SQL query performance: ``` name old time/op new time/op delta KV/Scan/SQL/rows=1-10 94.4µs ±10% 92.3µs ±11% -2.20% (p=0.000 n=93+93) KV/Scan/SQL/rows=10-10 102µs ±10% 99µs ±10% -2.16% (p=0.000 n=94+94) KV/Update/SQL/rows=10-10 378µs ±15% 370µs ±11% -2.04% (p=0.003 n=95+91) KV/Insert/SQL/rows=1-10 133µs ±14% 132µs ±12% ~ (p=0.738 n=95+93) KV/Insert/SQL/rows=10-10 197µs ±14% 196µs ±13% ~ (p=0.902 n=95+94) KV/Update/SQL/rows=1-10 186µs ±14% 185µs ±14% ~ (p=0.351 n=94+93) KV/Delete/SQL/rows=1-10 132µs ±13% 132µs ±14% ~ (p=0.473 n=94+94) KV/Delete/SQL/rows=10-10 254µs ±16% 250µs ±16% ~ (p=0.086 n=100+99) name old alloc/op new alloc/op delta KV/Scan/SQL/rows=1-10 20.1kB ± 0% 19.1kB ± 1% -4.91% (p=0.000 n=96+96) KV/Scan/SQL/rows=10-10 21.7kB ± 0% 20.7kB ± 1% -4.61% (p=0.000 n=96+97) KV/Delete/SQL/rows=10-10 64.0kB ± 3% 63.7kB ± 3% -0.55% (p=0.000 n=100+100) KV/Update/SQL/rows=1-10 45.8kB ± 1% 45.5kB ± 1% -0.55% (p=0.000 n=97+98) KV/Update/SQL/rows=10-10 105kB ± 1% 105kB ± 1% -0.10% (p=0.008 n=97+98) KV/Delete/SQL/rows=1-10 40.8kB ± 0% 40.7kB ± 0% -0.08% (p=0.001 n=95+96) KV/Insert/SQL/rows=1-10 37.4kB ± 1% 37.4kB ± 0% ~ (p=0.698 n=97+96) KV/Insert/SQL/rows=10-10 76.4kB ± 1% 76.4kB ± 0% ~ (p=0.822 n=99+98) name old allocs/op new allocs/op delta KV/Scan/SQL/rows=1-10 245 ± 0% 217 ± 0% -11.43% (p=0.000 n=95+92) KV/Scan/SQL/rows=10-10 280 ± 0% 252 ± 0% -10.11% (p=0.000 n=75+97) KV/Delete/SQL/rows=10-10 478 ± 0% 459 ± 0% -4.04% (p=0.000 n=94+97) KV/Delete/SQL/rows=1-10 297 ± 1% 287 ± 1% -3.34% (p=0.000 n=97+97) KV/Update/SQL/rows=1-10 459 ± 0% 444 ± 0% -3.27% (p=0.000 n=97+97) KV/Insert/SQL/rows=1-10 291 ± 0% 286 ± 0% -1.72% (p=0.000 n=82+86) KV/Update/SQL/rows=10-10 763 ± 1% 750 ± 1% -1.68% (p=0.000 n=96+98) KV/Insert/SQL/rows=10-10 489 ± 0% 484 ± 0% -1.03% (p=0.000 n=98+98) ``` 74340: sql: don't construct empty SessionData.CustomOptions r=otan a=nvanbenschoten This commit updates `newSessionData` to only construct a `CustomOptions` map if the `CustomOptionSessionDefaults` are not empty. The map is meant to be lazily allocated (see `sessionDataMutator.SetCustomOption`), so this ensures that it doesn't get created when it doesn't need to be. This change has two effects: 1. it avoids constructing a map in `newSessionData` 2. as a result, it also avoids the construction of an unnecessary map on each call to `SessionData.Clone`. The commit also adds additional protection in that method to avoid unnecessary map construction, but this is no longer strictly necessary with the change to `newSessionData`. ``` name old time/op new time/op delta KV/Scan/SQL/rows=1-10 95.3µs ± 6% 94.5µs ± 7% ~ (p=0.579 n=10+10) name old alloc/op new alloc/op delta KV/Scan/SQL/rows=1-10 20.1kB ± 0% 20.0kB ± 0% -0.34% (p=0.001 n=10+10) name old allocs/op new allocs/op delta KV/Scan/SQL/rows=1-10 245 ± 0% 244 ± 0% -0.41% (p=0.000 n=9+9) ``` ---- This is part of a collection of assorted micro-optimizations: - #74336 - #74337 - #74338 - #74339 - #74340 - #74341 - #74342 - #74343 - #74344 - #74345 - #74346 - #74347 - #74348 Combined, these changes have the following effect on end-to-end SQL query performance: ``` name old time/op new time/op delta KV/Scan/SQL/rows=1-10 94.4µs ±10% 92.3µs ±11% -2.20% (p=0.000 n=93+93) KV/Scan/SQL/rows=10-10 102µs ±10% 99µs ±10% -2.16% (p=0.000 n=94+94) KV/Update/SQL/rows=10-10 378µs ±15% 370µs ±11% -2.04% (p=0.003 n=95+91) KV/Insert/SQL/rows=1-10 133µs ±14% 132µs ±12% ~ (p=0.738 n=95+93) KV/Insert/SQL/rows=10-10 197µs ±14% 196µs ±13% ~ (p=0.902 n=95+94) KV/Update/SQL/rows=1-10 186µs ±14% 185µs ±14% ~ (p=0.351 n=94+93) KV/Delete/SQL/rows=1-10 132µs ±13% 132µs ±14% ~ (p=0.473 n=94+94) KV/Delete/SQL/rows=10-10 254µs ±16% 250µs ±16% ~ (p=0.086 n=100+99) name old alloc/op new alloc/op delta KV/Scan/SQL/rows=1-10 20.1kB ± 0% 19.1kB ± 1% -4.91% (p=0.000 n=96+96) KV/Scan/SQL/rows=10-10 21.7kB ± 0% 20.7kB ± 1% -4.61% (p=0.000 n=96+97) KV/Delete/SQL/rows=10-10 64.0kB ± 3% 63.7kB ± 3% -0.55% (p=0.000 n=100+100) KV/Update/SQL/rows=1-10 45.8kB ± 1% 45.5kB ± 1% -0.55% (p=0.000 n=97+98) KV/Update/SQL/rows=10-10 105kB ± 1% 105kB ± 1% -0.10% (p=0.008 n=97+98) KV/Delete/SQL/rows=1-10 40.8kB ± 0% 40.7kB ± 0% -0.08% (p=0.001 n=95+96) KV/Insert/SQL/rows=1-10 37.4kB ± 1% 37.4kB ± 0% ~ (p=0.698 n=97+96) KV/Insert/SQL/rows=10-10 76.4kB ± 1% 76.4kB ± 0% ~ (p=0.822 n=99+98) name old allocs/op new allocs/op delta KV/Scan/SQL/rows=1-10 245 ± 0% 217 ± 0% -11.43% (p=0.000 n=95+92) KV/Scan/SQL/rows=10-10 280 ± 0% 252 ± 0% -10.11% (p=0.000 n=75+97) KV/Delete/SQL/rows=10-10 478 ± 0% 459 ± 0% -4.04% (p=0.000 n=94+97) KV/Delete/SQL/rows=1-10 297 ± 1% 287 ± 1% -3.34% (p=0.000 n=97+97) KV/Update/SQL/rows=1-10 459 ± 0% 444 ± 0% -3.27% (p=0.000 n=97+97) KV/Insert/SQL/rows=1-10 291 ± 0% 286 ± 0% -1.72% (p=0.000 n=82+86) KV/Update/SQL/rows=10-10 763 ± 1% 750 ± 1% -1.68% (p=0.000 n=96+98) KV/Insert/SQL/rows=10-10 489 ± 0% 484 ± 0% -1.03% (p=0.000 n=98+98) ``` 74344: kv: move splitHealthy to method on grpcTransport r=tbg a=nvanbenschoten This commit moves the `splitHealthy` helper to a method on `grpcTransport`. This allows us to implement the `sort.Interface` interface on the heap-allocated `grpcTransport` and avoid the heap allocation previously incurred by `splitHealthy`. ``` name old time/op new time/op delta KV/Scan/Native/rows=1-10 14.9µs ± 4% 15.0µs ± 4% ~ (p=0.529 n=10+10) KV/Scan/SQL/rows=1-10 95.0µs ± 5% 94.5µs ± 7% ~ (p=0.393 n=10+10) name old alloc/op new alloc/op delta KV/Scan/Native/rows=1-10 6.87kB ± 0% 6.82kB ± 0% -0.71% (p=0.000 n=9+10) KV/Scan/SQL/rows=1-10 20.0kB ± 0% 20.0kB ± 0% -0.30% (p=0.007 n=10+10) name old allocs/op new allocs/op delta KV/Scan/Native/rows=1-10 52.0 ± 0% 51.0 ± 0% -1.92% (p=0.000 n=10+10) KV/Scan/SQL/rows=1-10 245 ± 0% 243 ± 0% -0.49% (p=0.001 n=10+10) ``` ---- This is part of a collection of assorted micro-optimizations: - #74336 - #74337 - #74338 - #74339 - #74340 - #74341 - #74342 - #74343 - #74344 - #74345 - #74346 - #74347 - #74348 Combined, these changes have the following effect on end-to-end SQL query performance: ``` name old time/op new time/op delta KV/Scan/SQL/rows=1-10 94.4µs ±10% 92.3µs ±11% -2.20% (p=0.000 n=93+93) KV/Scan/SQL/rows=10-10 102µs ±10% 99µs ±10% -2.16% (p=0.000 n=94+94) KV/Update/SQL/rows=10-10 378µs ±15% 370µs ±11% -2.04% (p=0.003 n=95+91) KV/Insert/SQL/rows=1-10 133µs ±14% 132µs ±12% ~ (p=0.738 n=95+93) KV/Insert/SQL/rows=10-10 197µs ±14% 196µs ±13% ~ (p=0.902 n=95+94) KV/Update/SQL/rows=1-10 186µs ±14% 185µs ±14% ~ (p=0.351 n=94+93) KV/Delete/SQL/rows=1-10 132µs ±13% 132µs ±14% ~ (p=0.473 n=94+94) KV/Delete/SQL/rows=10-10 254µs ±16% 250µs ±16% ~ (p=0.086 n=100+99) name old alloc/op new alloc/op delta KV/Scan/SQL/rows=1-10 20.1kB ± 0% 19.1kB ± 1% -4.91% (p=0.000 n=96+96) KV/Scan/SQL/rows=10-10 21.7kB ± 0% 20.7kB ± 1% -4.61% (p=0.000 n=96+97) KV/Delete/SQL/rows=10-10 64.0kB ± 3% 63.7kB ± 3% -0.55% (p=0.000 n=100+100) KV/Update/SQL/rows=1-10 45.8kB ± 1% 45.5kB ± 1% -0.55% (p=0.000 n=97+98) KV/Update/SQL/rows=10-10 105kB ± 1% 105kB ± 1% -0.10% (p=0.008 n=97+98) KV/Delete/SQL/rows=1-10 40.8kB ± 0% 40.7kB ± 0% -0.08% (p=0.001 n=95+96) KV/Insert/SQL/rows=1-10 37.4kB ± 1% 37.4kB ± 0% ~ (p=0.698 n=97+96) KV/Insert/SQL/rows=10-10 76.4kB ± 1% 76.4kB ± 0% ~ (p=0.822 n=99+98) name old allocs/op new allocs/op delta KV/Scan/SQL/rows=1-10 245 ± 0% 217 ± 0% -11.43% (p=0.000 n=95+92) KV/Scan/SQL/rows=10-10 280 ± 0% 252 ± 0% -10.11% (p=0.000 n=75+97) KV/Delete/SQL/rows=10-10 478 ± 0% 459 ± 0% -4.04% (p=0.000 n=94+97) KV/Delete/SQL/rows=1-10 297 ± 1% 287 ± 1% -3.34% (p=0.000 n=97+97) KV/Update/SQL/rows=1-10 459 ± 0% 444 ± 0% -3.27% (p=0.000 n=97+97) KV/Insert/SQL/rows=1-10 291 ± 0% 286 ± 0% -1.72% (p=0.000 n=82+86) KV/Update/SQL/rows=10-10 763 ± 1% 750 ± 1% -1.68% (p=0.000 n=96+98) KV/Insert/SQL/rows=10-10 489 ± 0% 484 ± 0% -1.03% (p=0.000 n=98+98) ``` 74346: kv: return pointers from TxnSender.GetLeafTxn{Input/Final}State r=tbg a=nvanbenschoten This commit changes `TxnSender`'s `GetLeafTxnInputState` and `GetLeafTxnFinalState` methods to return pointers instead of values. In `TxnCoordSender`'s implementation of these two methods, the structs were already escaping to the heap when passed to the `txnInterceptor` stack and the resulting structs were always being heap allocated by DistSQL. As a result, we were incurring two heap allocations by trying to return these structs by value. By heap allocating these structs immediately, we avoid an allocation and large memcpys. ``` name old time/op new time/op delta KV/Scan/SQL/rows=1-10 95.1µs ± 8% 95.8µs ± 5% ~ (p=0.393 n=10+10) KV/Scan/SQL/rows=10-10 100µs ± 2% 102µs ± 6% ~ (p=0.133 n=9+10) name old alloc/op new alloc/op delta KV/Scan/SQL/rows=1-10 20.1kB ± 0% 19.8kB ± 0% -1.46% (p=0.000 n=10+10) KV/Scan/SQL/rows=10-10 21.7kB ± 0% 21.4kB ± 0% -1.42% (p=0.000 n=9+10) name old allocs/op new allocs/op delta KV/Scan/SQL/rows=1-10 245 ± 0% 244 ± 0% -0.41% (p=0.002 n=8+10) KV/Scan/SQL/rows=10-10 280 ± 0% 279 ± 0% -0.36% (p=0.002 n=7+8) ``` ---- This is part of a collection of assorted micro-optimizations: - #74336 - #74337 - #74338 - #74339 - #74340 - #74341 - #74342 - #74343 - #74344 - #74345 - #74346 - #74347 - #74348 Combined, these changes have the following effect on end-to-end SQL query performance: ``` name old time/op new time/op delta KV/Scan/SQL/rows=1-10 94.4µs ±10% 92.3µs ±11% -2.20% (p=0.000 n=93+93) KV/Scan/SQL/rows=10-10 102µs ±10% 99µs ±10% -2.16% (p=0.000 n=94+94) KV/Update/SQL/rows=10-10 378µs ±15% 370µs ±11% -2.04% (p=0.003 n=95+91) KV/Insert/SQL/rows=1-10 133µs ±14% 132µs ±12% ~ (p=0.738 n=95+93) KV/Insert/SQL/rows=10-10 197µs ±14% 196µs ±13% ~ (p=0.902 n=95+94) KV/Update/SQL/rows=1-10 186µs ±14% 185µs ±14% ~ (p=0.351 n=94+93) KV/Delete/SQL/rows=1-10 132µs ±13% 132µs ±14% ~ (p=0.473 n=94+94) KV/Delete/SQL/rows=10-10 254µs ±16% 250µs ±16% ~ (p=0.086 n=100+99) name old alloc/op new alloc/op delta KV/Scan/SQL/rows=1-10 20.1kB ± 0% 19.1kB ± 1% -4.91% (p=0.000 n=96+96) KV/Scan/SQL/rows=10-10 21.7kB ± 0% 20.7kB ± 1% -4.61% (p=0.000 n=96+97) KV/Delete/SQL/rows=10-10 64.0kB ± 3% 63.7kB ± 3% -0.55% (p=0.000 n=100+100) KV/Update/SQL/rows=1-10 45.8kB ± 1% 45.5kB ± 1% -0.55% (p=0.000 n=97+98) KV/Update/SQL/rows=10-10 105kB ± 1% 105kB ± 1% -0.10% (p=0.008 n=97+98) KV/Delete/SQL/rows=1-10 40.8kB ± 0% 40.7kB ± 0% -0.08% (p=0.001 n=95+96) KV/Insert/SQL/rows=1-10 37.4kB ± 1% 37.4kB ± 0% ~ (p=0.698 n=97+96) KV/Insert/SQL/rows=10-10 76.4kB ± 1% 76.4kB ± 0% ~ (p=0.822 n=99+98) name old allocs/op new allocs/op delta KV/Scan/SQL/rows=1-10 245 ± 0% 217 ± 0% -11.43% (p=0.000 n=95+92) KV/Scan/SQL/rows=10-10 280 ± 0% 252 ± 0% -10.11% (p=0.000 n=75+97) KV/Delete/SQL/rows=10-10 478 ± 0% 459 ± 0% -4.04% (p=0.000 n=94+97) KV/Delete/SQL/rows=1-10 297 ± 1% 287 ± 1% -3.34% (p=0.000 n=97+97) KV/Update/SQL/rows=1-10 459 ± 0% 444 ± 0% -3.27% (p=0.000 n=97+97) KV/Insert/SQL/rows=1-10 291 ± 0% 286 ± 0% -1.72% (p=0.000 n=82+86) KV/Update/SQL/rows=10-10 763 ± 1% 750 ± 1% -1.68% (p=0.000 n=96+98) KV/Insert/SQL/rows=10-10 489 ± 0% 484 ± 0% -1.03% (p=0.000 n=98+98) ``` 74347: kv: inline small condensableSpanSet slices r=tbg a=nvanbenschoten This commit adds a small pre-allocated array of spans to `condensableSpanSet` that is used to avoid heap allocations for transactions with a small number of refresh spans and a small number of lock spans. ``` name old time/op new time/op delta KV/Scan/SQL/rows=1-10 95.1µs ± 7% 95.9µs ± 5% ~ (p=0.579 n=10+10) KV/Scan/SQL/rows=10-10 100µs ± 3% 103µs ±12% ~ (p=0.829 n=8+10) name old alloc/op new alloc/op delta KV/Scan/SQL/rows=10-10 21.7kB ± 0% 21.5kB ± 0% -0.76% (p=0.000 n=10+10) KV/Scan/SQL/rows=1-10 20.1kB ± 0% 19.9kB ± 0% -0.70% (p=0.000 n=10+9) name old allocs/op new allocs/op delta KV/Scan/SQL/rows=1-10 245 ± 0% 244 ± 0% -0.41% (p=0.000 n=10+10) KV/Scan/SQL/rows=10-10 280 ± 0% 279 ± 0% -0.36% (p=0.001 n=8+9) ``` ---- This is part of a collection of assorted micro-optimizations: - #74336 - #74337 - #74338 - #74339 - #74340 - #74341 - #74342 - #74343 - #74344 - #74345 - #74346 - #74347 - #74348 Combined, these changes have the following effect on end-to-end SQL query performance: ``` name old time/op new time/op delta KV/Scan/SQL/rows=1-10 94.4µs ±10% 92.3µs ±11% -2.20% (p=0.000 n=93+93) KV/Scan/SQL/rows=10-10 102µs ±10% 99µs ±10% -2.16% (p=0.000 n=94+94) KV/Update/SQL/rows=10-10 378µs ±15% 370µs ±11% -2.04% (p=0.003 n=95+91) KV/Insert/SQL/rows=1-10 133µs ±14% 132µs ±12% ~ (p=0.738 n=95+93) KV/Insert/SQL/rows=10-10 197µs ±14% 196µs ±13% ~ (p=0.902 n=95+94) KV/Update/SQL/rows=1-10 186µs ±14% 185µs ±14% ~ (p=0.351 n=94+93) KV/Delete/SQL/rows=1-10 132µs ±13% 132µs ±14% ~ (p=0.473 n=94+94) KV/Delete/SQL/rows=10-10 254µs ±16% 250µs ±16% ~ (p=0.086 n=100+99) name old alloc/op new alloc/op delta KV/Scan/SQL/rows=1-10 20.1kB ± 0% 19.1kB ± 1% -4.91% (p=0.000 n=96+96) KV/Scan/SQL/rows=10-10 21.7kB ± 0% 20.7kB ± 1% -4.61% (p=0.000 n=96+97) KV/Delete/SQL/rows=10-10 64.0kB ± 3% 63.7kB ± 3% -0.55% (p=0.000 n=100+100) KV/Update/SQL/rows=1-10 45.8kB ± 1% 45.5kB ± 1% -0.55% (p=0.000 n=97+98) KV/Update/SQL/rows=10-10 105kB ± 1% 105kB ± 1% -0.10% (p=0.008 n=97+98) KV/Delete/SQL/rows=1-10 40.8kB ± 0% 40.7kB ± 0% -0.08% (p=0.001 n=95+96) KV/Insert/SQL/rows=1-10 37.4kB ± 1% 37.4kB ± 0% ~ (p=0.698 n=97+96) KV/Insert/SQL/rows=10-10 76.4kB ± 1% 76.4kB ± 0% ~ (p=0.822 n=99+98) name old allocs/op new allocs/op delta KV/Scan/SQL/rows=1-10 245 ± 0% 217 ± 0% -11.43% (p=0.000 n=95+92) KV/Scan/SQL/rows=10-10 280 ± 0% 252 ± 0% -10.11% (p=0.000 n=75+97) KV/Delete/SQL/rows=10-10 478 ± 0% 459 ± 0% -4.04% (p=0.000 n=94+97) KV/Delete/SQL/rows=1-10 297 ± 1% 287 ± 1% -3.34% (p=0.000 n=97+97) KV/Update/SQL/rows=1-10 459 ± 0% 444 ± 0% -3.27% (p=0.000 n=97+97) KV/Insert/SQL/rows=1-10 291 ± 0% 286 ± 0% -1.72% (p=0.000 n=82+86) KV/Update/SQL/rows=10-10 763 ± 1% 750 ± 1% -1.68% (p=0.000 n=96+98) KV/Insert/SQL/rows=10-10 489 ± 0% 484 ± 0% -1.03% (p=0.000 n=98+98) ``` 74350: kv: avoid retry error allocation on each Txn.exec call r=tbg a=nvanbenschoten Similar to #74339. This commit updates `Txn.exec` to only call `errors.As` if the error from the transaction execution was non-nil. This avoids a heap allocation of a `TransactionRetryWithProtoRefreshError` on each call to `Txn.exec`, which was originally introduced in a8ae1bf. In kv100 (100% reads), this is responsible for **0.36%** of total CPU usage. <img width="1586" alt="Screen Shot 2021-12-30 at 6 16 57 PM" src="https://user-images.githubusercontent.com/5438456/147793901-7a5c5d6e-8022-4c98-a349-88ed2def62d6.png"> ---- This is part of a collection of assorted micro-optimizations: - #74336 - #74337 - #74338 - #74339 - #74340 - #74341 - #74342 - #74343 - #74344 - #74345 - #74346 - #74347 - #74348 Combined, these changes have the following effect on end-to-end SQL query performance: ``` name old time/op new time/op delta KV/Scan/SQL/rows=1-10 94.4µs ±10% 92.3µs ±11% -2.20% (p=0.000 n=93+93) KV/Scan/SQL/rows=10-10 102µs ±10% 99µs ±10% -2.16% (p=0.000 n=94+94) KV/Update/SQL/rows=10-10 378µs ±15% 370µs ±11% -2.04% (p=0.003 n=95+91) KV/Insert/SQL/rows=1-10 133µs ±14% 132µs ±12% ~ (p=0.738 n=95+93) KV/Insert/SQL/rows=10-10 197µs ±14% 196µs ±13% ~ (p=0.902 n=95+94) KV/Update/SQL/rows=1-10 186µs ±14% 185µs ±14% ~ (p=0.351 n=94+93) KV/Delete/SQL/rows=1-10 132µs ±13% 132µs ±14% ~ (p=0.473 n=94+94) KV/Delete/SQL/rows=10-10 254µs ±16% 250µs ±16% ~ (p=0.086 n=100+99) name old alloc/op new alloc/op delta KV/Scan/SQL/rows=1-10 20.1kB ± 0% 19.1kB ± 1% -4.91% (p=0.000 n=96+96) KV/Scan/SQL/rows=10-10 21.7kB ± 0% 20.7kB ± 1% -4.61% (p=0.000 n=96+97) KV/Delete/SQL/rows=10-10 64.0kB ± 3% 63.7kB ± 3% -0.55% (p=0.000 n=100+100) KV/Update/SQL/rows=1-10 45.8kB ± 1% 45.5kB ± 1% -0.55% (p=0.000 n=97+98) KV/Update/SQL/rows=10-10 105kB ± 1% 105kB ± 1% -0.10% (p=0.008 n=97+98) KV/Delete/SQL/rows=1-10 40.8kB ± 0% 40.7kB ± 0% -0.08% (p=0.001 n=95+96) KV/Insert/SQL/rows=1-10 37.4kB ± 1% 37.4kB ± 0% ~ (p=0.698 n=97+96) KV/Insert/SQL/rows=10-10 76.4kB ± 1% 76.4kB ± 0% ~ (p=0.822 n=99+98) name old allocs/op new allocs/op delta KV/Scan/SQL/rows=1-10 245 ± 0% 217 ± 0% -11.43% (p=0.000 n=95+92) KV/Scan/SQL/rows=10-10 280 ± 0% 252 ± 0% -10.11% (p=0.000 n=75+97) KV/Delete/SQL/rows=10-10 478 ± 0% 459 ± 0% -4.04% (p=0.000 n=94+97) KV/Delete/SQL/rows=1-10 297 ± 1% 287 ± 1% -3.34% (p=0.000 n=97+97) KV/Update/SQL/rows=1-10 459 ± 0% 444 ± 0% -3.27% (p=0.000 n=97+97) KV/Insert/SQL/rows=1-10 291 ± 0% 286 ± 0% -1.72% (p=0.000 n=82+86) KV/Update/SQL/rows=10-10 763 ± 1% 750 ± 1% -1.68% (p=0.000 n=96+98) KV/Insert/SQL/rows=10-10 489 ± 0% 484 ± 0% -1.03% (p=0.000 n=98+98) ``` 74353: sql/schema: re-organize the UnleasableSystemDescriptors set r=ajwerner a=nvanbenschoten This commit re-organizes the static maps we use to perform lookups into the `UnleasableSystemDescriptors`. It splits the `UnleasableSystemDescriptors` map into two, one optimized for `leasedDescriptors.getByName` and one optimized for `leasedDescriptors.getByID`. In a 100% read workload, `leasedDescriptors` accesses (`getByName` and `getByID`) were responsible for **4.39** of total CPU utilization. <img width="1578" alt="Screen Shot 2021-12-30 at 7 35 38 PM" src="https://user-images.githubusercontent.com/5438456/147796756-5d5c4d94-ebfe-435d-983d-eb0263c18e1c.png"> Within this, about **0.33%** of total CPU utilization was spent directly in these functions. This change shouldn't make a large difference (less than **0.2%**), but it should improve things slightly. Since it also improves readability, it seems worthwhile. ``` File: cockroach Type: cpu Time: Dec 30, 2021 at 10:57pm (UTC) Duration: 30.15s, Total samples = 74.99s (248.72%) Active filters: focus=leasedDescriptors\).getBy Showing nodes accounting for 3.29s, 4.39% of 74.99s total ----------------------------------------------------------+------------- flat flat% sum% cum cum% calls calls% + context ----------------------------------------------------------+------------- 0.14s 100% | github.com/cockroachdb/cockroach/pkg/sql/catalog/descs.(*Collection).getByName /go/src/github.com/cockroachdb/cockroach/pkg/sql/catalog/descs/descriptor.go:199 0.10s 0.13% 0.13% 0.14s 0.19% | github.com/cockroachdb/cockroach/pkg/sql/catalog/descs.(*leasedDescriptors).getByName /go/src/github.com/cockroachdb/cockroach/pkg/sql/catalog/descs/leased_descriptors.go:102 0.03s 21.43% | github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb.(*TableDescriptor).GetParentID /go/src/github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb/structured.pb.go:2347 0.01s 7.14% | github.com/cockroachdb/cockroach/pkg/sql/catalog/dbdesc.(*immutable).GetParentID /go/src/github.com/cockroachdb/cockroach/pkg/sql/catalog/dbdesc/database_desc.go:93 ----------------------------------------------------------+------------- 0.14s 100% | github.com/cockroachdb/cockroach/pkg/sql/catalog/descs.(*Collection).getByName /go/src/github.com/cockroachdb/cockroach/pkg/sql/catalog/descs/descriptor.go:199 0.04s 0.053% 0.19% 0.14s 0.19% | github.com/cockroachdb/cockroach/pkg/sql/catalog/descs.(*leasedDescriptors).getByName /go/src/github.com/cockroachdb/cockroach/pkg/sql/catalog/descs/leased_descriptors.go:101 0.03s 21.43% | runtime.mapiternext /usr/local/go/src/runtime/map.go:851 0.02s 14.29% | runtime.mapiterinit /usr/local/go/src/runtime/map.go:821 0.01s 7.14% | runtime.duffzero /usr/local/go/src/runtime/duff_amd64.s:95 0.01s 7.14% | runtime.mapiterinit /usr/local/go/src/runtime/map.go:832 0.01s 7.14% | runtime.mapiterinit /usr/local/go/src/runtime/map.go:848 0.01s 7.14% | runtime.mapiternext /usr/local/go/src/runtime/map.go:898 0.01s 7.14% | runtime.mapiternext /usr/local/go/src/runtime/map.go:972 ----------------------------------------------------------+------------- ``` 74356: kv: acquire Replica shared mutex in tryGetOrCreateReplica r=tbg a=nvanbenschoten This commit switches the common path in `tryGetOrCreateReplica` from grabbing the Replica mutex in an exclusive mode to grabbing it in a shared mode. The common path in `tryGetOrCreateReplica` does not mutate the Replica, so there's no need for the stronger lock. The locking in `setLastReplicaDescriptors` showed up in a mutex profile under a write-heavy workload. It was responsible for **2.06%** of mutex wait time. Grabbing the mutex was probably also slowing down Raft request processing, as the exclusive lock acquisition had to wait for other read locks to be dropped. <img width="1584" alt="Screen Shot 2021-12-30 at 9 45 26 PM" src="https://user-images.githubusercontent.com/5438456/147800611-e3ff4008-56c4-4fed-810b-bb85c50de522.png"> 74358: kv: combine heap allocations in maybeStripInFlightWrites r=tbg a=nvanbenschoten 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 ----------------------------------------------------------+------------- ``` Co-authored-by: Nathan VanBenschoten <[email protected]>
cockroachdb · Jan 3, 2022 · 5669b9d · 5669b9d
12 parents bc88633 + 79a5230 + 47900eb + 840ac21 + 5ecb6e2 + 71cb6f5 + 03a4fad + d9e17f0 + 1f021e4 + 362242f + 36e3163 + 8fc9aa9
commit 5669b9d
Show file tree

Hide file tree

Showing 29 changed files with 243 additions and 153 deletions.
diff --git a/pkg/kv/kvclient/kvcoord/condensable_span_set.go b/pkg/kv/kvclient/kvcoord/condensable_span_set.go
@@ -36,11 +36,17 @@ type condensableSpanSet struct {
 	// memory without losing fidelity, in which case this flag would not be set
 	// (e.g. merging overlapping or adjacent spans).
 	condensed bool
+
+	// Avoid heap allocations for transactions with a small number of spans.
+	sAlloc [2]roachpb.Span
 }
 
 // insert adds new spans to the condensable span set. No attempt to condense the
 // set or deduplicate the new span with existing spans is made.
 func (s *condensableSpanSet) insert(spans ...roachpb.Span) {
+	if cap(s.s) == 0 {
+		s.s = s.sAlloc[:0]
+	}
 	s.s = append(s.s, spans...)
 	for _, sp := range spans {
 		s.bytes += spanSize(sp)

diff --git a/pkg/kv/kvclient/kvcoord/send_test.go b/pkg/kv/kvclient/kvcoord/send_test.go
@@ -245,8 +245,8 @@ func TestComplexScenarios(t *testing.T) {
 	}
 }
 
-// TestSplitHealthy tests that the splitHealthy helper function sorts healthy
-// nodes before unhealthy nodes.
+// TestSplitHealthy tests that the splitHealthy method sorts healthy nodes
+// before unhealthy nodes.
 func TestSplitHealthy(t *testing.T) {
 	defer leaktest.AfterTest(t)()
 	defer log.Scope(t).Close(t)
@@ -299,8 +299,12 @@ func TestSplitHealthy(t *testing.T) {
 					health.Set(i, healthUnhealthy)
 				}
 			}
-			splitHealthy(replicas, health)
-			if !reflect.DeepEqual(replicas, td.out) {
+			gt := grpcTransport{
+				replicas:      replicas,
+				replicaHealth: health,
+			}
+			gt.splitHealthy()
+			if !reflect.DeepEqual(gt.replicas, td.out) {
 				t.Errorf("splitHealthy(...) = %+v not %+v", replicas, td.out)
 			}
 		})

diff --git a/pkg/kv/kvclient/kvcoord/transport.go b/pkg/kv/kvclient/kvcoord/transport.go
@@ -117,7 +117,8 @@ func grpcTransportFactoryImpl(
 
 	// We'll map the index of the replica descriptor in its slice to its health.
 	var health util.FastIntMap
-	for i, r := range rs {
+	for i := range rs {
+		r := &rs[i]
 		replicas[i] = r.ReplicaDescriptor
 		healthy := nodeDialer.ConnHealth(r.NodeID, opts.class) == nil
 		if healthy {
@@ -127,18 +128,20 @@ func grpcTransportFactoryImpl(
 		}
 	}
 
+	*transport = grpcTransport{
+		opts:          opts,
+		nodeDialer:    nodeDialer,
+		class:         opts.class,
+		replicas:      replicas,
+		replicaHealth: health,
+	}
+
 	if !opts.dontConsiderConnHealth {
-		// Put known-healthy clients first, while otherwise respecting the existing
+		// Put known-healthy replica first, while otherwise respecting the existing
 		// ordering of the replicas.
-		splitHealthy(replicas, health)
+		transport.splitHealthy()
 	}
 
-	*transport = grpcTransport{
-		opts:       opts,
-		nodeDialer: nodeDialer,
-		class:      opts.class,
-		replicas:   replicas,
-	}
 	return transport, nil
 }
 
@@ -148,6 +151,9 @@ type grpcTransport struct {
 	class      rpc.ConnectionClass
 
 	replicas []roachpb.ReplicaDescriptor
+	// replicaHealth maps replica index within the replicas slice to healthHealthy
+	// if healthy, and healthUnhealthy if unhealthy. Used by splitHealthy.
+	replicaHealth util.FastIntMap
 	// nextReplicaIdx represents the index into replicas of the next replica to be
 	// tried.
 	nextReplicaIdx int
@@ -262,38 +268,30 @@ func (gt *grpcTransport) MoveToFront(replica roachpb.ReplicaDescriptor) {
 	}
 }
 
-// splitHealthy splits the provided client slice into healthy clients and
-// unhealthy clients, based on their connection state. Healthy clients will
-// be rearranged first in the slice, and unhealthy clients will be rearranged
-// last. Within these two groups, the rearrangement will be stable. The function
-// will then return the number of healthy clients.
-// The input FastIntMap maps index within the input replicas slice to an integer
-// healthHealthy or healthUnhealthy.
-func splitHealthy(replicas []roachpb.ReplicaDescriptor, health util.FastIntMap) {
-	sort.Stable(&byHealth{replicas: replicas, health: health})
+// splitHealthy splits the grpcTransport's replica slice into healthy replica
+// and unhealthy replica, based on their connection state. Healthy replicas will
+// be rearranged first in the replicas slice, and unhealthy replicas will be
+// rearranged last. Within these two groups, the rearrangement will be stable.
+func (gt *grpcTransport) splitHealthy() {
+	sort.Stable((*byHealth)(gt))
 }
 
-// byHealth sorts a slice of batchClients by their health with healthy first.
-type byHealth struct {
-	replicas []roachpb.ReplicaDescriptor
-	// This map maps replica index within the replicas slice to healthHealthy if
-	// healthy, and healthUnhealthy if unhealthy.
-	health util.FastIntMap
-}
+// byHealth sorts a slice of replicas by their health with healthy first.
+type byHealth grpcTransport
 
 func (h *byHealth) Len() int { return len(h.replicas) }
 func (h *byHealth) Swap(i, j int) {
 	h.replicas[i], h.replicas[j] = h.replicas[j], h.replicas[i]
-	oldI := h.health.GetDefault(i)
-	h.health.Set(i, h.health.GetDefault(j))
-	h.health.Set(j, oldI)
+	oldI := h.replicaHealth.GetDefault(i)
+	h.replicaHealth.Set(i, h.replicaHealth.GetDefault(j))
+	h.replicaHealth.Set(j, oldI)
 }
 func (h *byHealth) Less(i, j int) bool {
-	ih, ok := h.health.Get(i)
+	ih, ok := h.replicaHealth.Get(i)
 	if !ok {
 		panic(fmt.Sprintf("missing health info for %s", h.replicas[i]))
 	}
-	jh, ok := h.health.Get(j)
+	jh, ok := h.replicaHealth.Get(j)
 	if !ok {
 		panic(fmt.Sprintf("missing health info for %s", h.replicas[j]))
 	}

diff --git a/pkg/kv/kvclient/kvcoord/txn_coord_sender.go b/pkg/kv/kvclient/kvcoord/txn_coord_sender.go
@@ -1169,19 +1169,19 @@ func (tc *TxnCoordSender) Active() bool {
 // GetLeafTxnInputState is part of the client.TxnSender interface.
 func (tc *TxnCoordSender) GetLeafTxnInputState(
 	ctx context.Context, opt kv.TxnStatusOpt,
-) (roachpb.LeafTxnInputState, error) {
+) (*roachpb.LeafTxnInputState, error) {
+	tis := new(roachpb.LeafTxnInputState)
 	tc.mu.Lock()
 	defer tc.mu.Unlock()
 
 	if err := tc.checkTxnStatusLocked(ctx, opt); err != nil {
-		return roachpb.LeafTxnInputState{}, err
+		return nil, err
 	}
 
 	// Copy mutable state so access is safe for the caller.
-	var tis roachpb.LeafTxnInputState
 	tis.Txn = tc.mu.txn
 	for _, reqInt := range tc.interceptorStack {
-		reqInt.populateLeafInputState(&tis)
+		reqInt.populateLeafInputState(tis)
 	}
 
 	// Also mark the TxnCoordSender as "active".  This prevents changing
@@ -1196,16 +1196,15 @@ func (tc *TxnCoordSender) GetLeafTxnInputState(
 // GetLeafTxnFinalState is part of the client.TxnSender interface.
 func (tc *TxnCoordSender) GetLeafTxnFinalState(
 	ctx context.Context, opt kv.TxnStatusOpt,
-) (roachpb.LeafTxnFinalState, error) {
+) (*roachpb.LeafTxnFinalState, error) {
+	tfs := new(roachpb.LeafTxnFinalState)
 	tc.mu.Lock()
 	defer tc.mu.Unlock()
 
 	if err := tc.checkTxnStatusLocked(ctx, opt); err != nil {
-		return roachpb.LeafTxnFinalState{}, err
+		return nil, err
 	}
 
-	var tfs roachpb.LeafTxnFinalState
-
 	// For compatibility with pre-20.1 nodes: populate the command
 	// count.
 	// TODO(knz,andrei): Remove this and the command count
@@ -1217,7 +1216,7 @@ func (tc *TxnCoordSender) GetLeafTxnFinalState(
 	// Copy mutable state so access is safe for the caller.
 	tfs.Txn = tc.mu.txn
 	for _, reqInt := range tc.interceptorStack {
-		reqInt.populateLeafFinalState(&tfs)
+		reqInt.populateLeafFinalState(tfs)
 	}
 
 	return tfs, nil

diff --git a/pkg/kv/kvclient/kvcoord/txn_coord_sender_test.go b/pkg/kv/kvclient/kvcoord/txn_coord_sender_test.go
@@ -852,7 +852,7 @@ func TestTxnMultipleCoord(t *testing.T) {
 
 	// New create a second, leaf coordinator.
 	leafInputState := txn.GetLeafTxnInputState(ctx)
-	txn2 := kv.NewLeafTxn(ctx, s.DB, 0 /* gatewayNodeID */, &leafInputState)
+	txn2 := kv.NewLeafTxn(ctx, s.DB, 0 /* gatewayNodeID */, leafInputState)
 
 	// Start the second transaction.
 	key2 := roachpb.Key("b")
@@ -865,7 +865,7 @@ func TestTxnMultipleCoord(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	if err := txn.UpdateRootWithLeafFinalState(ctx, &tfs); err != nil {
+	if err := txn.UpdateRootWithLeafFinalState(ctx, tfs); err != nil {
 		t.Fatal(err)
 	}
 
@@ -2426,7 +2426,7 @@ func TestLeafTxnClientRejectError(t *testing.T) {
 	leafInputState := rootTxn.GetLeafTxnInputState(ctx)
 
 	// New create a second, leaf coordinator.
-	leafTxn := kv.NewLeafTxn(ctx, s.DB, 0 /* gatewayNodeID */, &leafInputState)
+	leafTxn := kv.NewLeafTxn(ctx, s.DB, 0 /* gatewayNodeID */, leafInputState)
 
 	if _, err := leafTxn.Get(ctx, errKey); !testutils.IsError(err, "TransactionAbortedError") {
 		t.Fatalf("expected injected err, got: %v", err)
@@ -2455,14 +2455,14 @@ func TestUpdateRoootWithLeafFinalStateInAbortedTxn(t *testing.T) {
 
 	txn := kv.NewTxn(ctx, s.DB, 0 /* gatewayNodeID */)
 	leafInputState := txn.GetLeafTxnInputState(ctx)
-	leafTxn := kv.NewLeafTxn(ctx, s.DB, 0, &leafInputState)
+	leafTxn := kv.NewLeafTxn(ctx, s.DB, 0, leafInputState)
 
 	finalState, err := leafTxn.GetLeafTxnFinalState(ctx)
 	if err != nil {
 		t.Fatal(err)
 	}
 	finalState.Txn.Status = roachpb.ABORTED
-	if err := txn.UpdateRootWithLeafFinalState(ctx, &finalState); err != nil {
+	if err := txn.UpdateRootWithLeafFinalState(ctx, finalState); err != nil {
 		t.Fatal(err)
 	}
 

diff --git 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

diff --git a/pkg/kv/kvserver/store_create_replica.go b/pkg/kv/kvserver/store_create_replica.go
@@ -81,18 +81,18 @@ func (s *Store) tryGetOrCreateReplica(
 	// The common case: look up an existing (initialized) replica.
 	if repl, ok := s.mu.replicasByRangeID.Load(rangeID); ok {
 		repl.raftMu.Lock() // not unlocked on success
-		repl.mu.Lock()
+		repl.mu.RLock()
 
 		// The current replica is removed, go back around.
 		if repl.mu.destroyStatus.Removed() {
-			repl.mu.Unlock()
+			repl.mu.RUnlock()
 			repl.raftMu.Unlock()
 			return nil, false, errRetry
 		}
 
 		// Drop messages from replicas we know to be too old.
-		if fromReplicaIsTooOld(repl, creatingReplica) {
-			repl.mu.Unlock()
+		if fromReplicaIsTooOldRLocked(repl, creatingReplica) {
+			repl.mu.RUnlock()
 			repl.raftMu.Unlock()
 			return nil, false, roachpb.NewReplicaTooOldError(creatingReplica.ReplicaID)
 		}
@@ -104,7 +104,7 @@ func (s *Store) tryGetOrCreateReplica(
 					replicaID, repl)
 			}
 
-			repl.mu.Unlock()
+			repl.mu.RUnlock()
 			if err := s.removeReplicaRaftMuLocked(ctx, repl, replicaID, RemoveOptions{
 				DestroyData: true,
 			}); err != nil {
@@ -113,7 +113,7 @@ func (s *Store) tryGetOrCreateReplica(
 			repl.raftMu.Unlock()
 			return nil, false, errRetry
 		}
-		defer repl.mu.Unlock()
+		defer repl.mu.RUnlock()
 
 		if repl.mu.replicaID > replicaID {
 			// The sender is behind and is sending to an old replica.
@@ -238,11 +238,11 @@ func (s *Store) tryGetOrCreateReplica(
 	return repl, true, nil
 }
 
-// isFromReplicaTooOld returns an true if the creatingReplica is deemed to be
-// a member of the range which has been removed.
-// Assumes toReplica.mu is held.
-func fromReplicaIsTooOld(toReplica *Replica, fromReplica *roachpb.ReplicaDescriptor) bool {
-	toReplica.mu.AssertHeld()
+// fromReplicaIsTooOldRLocked returns true if the creatingReplica is deemed to
+// be a member of the range which has been removed.
+// Assumes toReplica.mu is locked for (at least) reading.
+func fromReplicaIsTooOldRLocked(toReplica *Replica, fromReplica *roachpb.ReplicaDescriptor) bool {
+	toReplica.mu.AssertRHeld()
 	if fromReplica == nil {
 		return false
 	}

diff --git a/pkg/kv/mock_transactional_sender.go b/pkg/kv/mock_transactional_sender.go
@@ -48,14 +48,14 @@ func (m *MockTransactionalSender) Send(
 // GetLeafTxnInputState is part of the TxnSender interface.
 func (m *MockTransactionalSender) GetLeafTxnInputState(
 	context.Context, TxnStatusOpt,
-) (roachpb.LeafTxnInputState, error) {
+) (*roachpb.LeafTxnInputState, error) {
 	panic("unimplemented")
 }
 
 // GetLeafTxnFinalState is part of the TxnSender interface.
 func (m *MockTransactionalSender) GetLeafTxnFinalState(
 	context.Context, TxnStatusOpt,
-) (roachpb.LeafTxnFinalState, error) {
+) (*roachpb.LeafTxnFinalState, error) {
 	panic("unimplemented")
 }
 

diff --git a/pkg/kv/sender.go b/pkg/kv/sender.go
@@ -112,12 +112,12 @@ type TxnSender interface {
 	//
 	// If AnyTxnStatus is passed, then this function never returns
 	// errors.
-	GetLeafTxnInputState(context.Context, TxnStatusOpt) (roachpb.LeafTxnInputState, error)
+	GetLeafTxnInputState(context.Context, TxnStatusOpt) (*roachpb.LeafTxnInputState, error)
 
 	// GetLeafTxnFinalState retrieves the final state of a LeafTxn
 	// necessary and sufficient to update a RootTxn with progress made
 	// on its behalf by the LeafTxn.
-	GetLeafTxnFinalState(context.Context, TxnStatusOpt) (roachpb.LeafTxnFinalState, error)
+	GetLeafTxnFinalState(context.Context, TxnStatusOpt) (*roachpb.LeafTxnFinalState, error)
 
 	// UpdateRootWithLeafFinalState updates a RootTxn using the final
 	// state of a LeafTxn.