Merge #74348

74348: sql: release BatchFlowCoordinator objects to pool r=nvanbenschoten a=nvanbenschoten

This commit adds `BatchFlowCoordinator` objects to their `vectorizedFlowCreator`'s
`releasables` slice, which ensures that their `Release` method is called and that
they are properly recycled.

Before this change, heap profiles and instrumentation both indicated that these
objects were not being recycled as intended. For example, heap profiles looked like:
```
      Type: alloc_objects
Time: Dec 30, 2021 at 2:10pm (EST)
Active filters:
   focus=Pool\).Get
Showing nodes accounting for 92189, 0.57% of 16223048 total
----------------------------------------------------------+-------------
      flat  flat%   sum%        cum   cum%   calls calls% + context
----------------------------------------------------------+-------------
                                             71505 79.69% |   github.com/cockroachdb/cockroach/pkg/sql/colflow.NewBatchFlowCoordinator /Users/nathan/Go/src/github.com/cockroachdb/cockroach/pkg/sql/colflow/flow_coordinator.go:218
                                             10923 12.17% |   github.com/cockroachdb/cockroach/pkg/kv/kvserver/concurrency.newLockTableGuardImpl /Users/nathan/Go/src/github.com/cockroachdb/cockroach/pkg/kv/kvserver/concurrency/lock_table.go:452
                                              2048  2.28% |   github.com/cockroachdb/cockroach/pkg/sql/catalog/nstree.(*Map).maybeInitialize /Users/nathan/Go/src/github.com/cockroachdb/cockroach/pkg/sql/catalog/nstree/map.go:112
                                              1170  1.30% |   github.com/cockroachdb/cockroach/pkg/sql/colexec.newMaterializerInternal /Users/nathan/Go/src/github.com/cockroachdb/cockroach/pkg/sql/colexec/materializer.go:179
                                              1030  1.15% |   github.com/cockroachdb/pebble.(*Iterator).Clone /Users/nathan/Go/src/github.com/cockroachdb/cockroach/vendor/github.com/cockroachdb/pebble/iterator.go:1228
                                               585  0.65% |   github.com/cockroachdb/cockroach/pkg/sql.MakeDistSQLReceiver /Users/nathan/Go/src/github.com/cockroachdb/cockroach/pkg/sql/distsql_running.go:841
                                               585  0.65% |   github.com/cockroachdb/cockroach/pkg/sql/colfetcher.NewColBatchScan /Users/nathan/Go/src/github.com/cockroachdb/cockroach/pkg/sql/colfetcher/colbatch_scan.go:223
                                               585  0.65% |   github.com/cockroachdb/cockroach/pkg/sql/span.MakeBuilder /Users/nathan/Go/src/github.com/cockroachdb/cockroach/pkg/sql/span/span_builder.go:61
                                               585  0.65% |   github.com/cockroachdb/cockroach/pkg/storage.mvccGet /Users/nathan/Go/src/github.com/cockroachdb/cockroach/pkg/storage/mvcc.go:859
                                               585  0.65% |   github.com/cockroachdb/cockroach/pkg/storage.mvccScanToBytes /Users/nathan/Go/src/github.com/cockroachdb/cockroach/pkg/storage/mvcc.go:2357
                                               128  0.14% |   github.com/cockroachdb/pebble.(*DB).newIterInternal /Users/nathan/Go/src/github.com/cockroachdb/cockroach/vendor/github.com/cockroachdb/pebble/db.go:765
         0     0%     0%      89729  0.55%                | sync.(*Pool).Get /Users/nathan/Go/go/src/sync/pool.go:148
```
Notice that `sync.(*Pool).Get` is responsible for *0.55%* of heap allocations and
**79.69%** of these are from `colflow.NewBatchFlowCoordinator`.

After this change, that source of allocations goes away and we see the following
impact on micro-benchmarks:
```
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)
```


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

pkg/sql/colflow/vectorized_flow.go

@@ -993,6 +993,7 @@ func (s *vectorizedFlowCreator) setupOutput(
 			)
 			// The flow coordinator is a root of its operator chain.
 			s.opChains = append(s.opChains, s.batchFlowCoordinator)
+			s.releasables = append(s.releasables, s.batchFlowCoordinator)
 		} else {
 			// We need to use the row receiving output.
 			if input != nil {
@@ -1021,6 +1022,9 @@ func (s *vectorizedFlowCreator) setupOutput(
 			)
 			// The flow coordinator is a root of its operator chain.
 			s.opChains = append(s.opChains, f)
+			// NOTE: we don't append f to s.releasables because addFlowCoordinator
+			// adds the FlowCoordinator to FlowBase.processors, which ensures that
+			// it is later released in FlowBase.Cleanup.
 			s.addFlowCoordinator(f)
 		}