sql: remove a bunch of allocations from name and type resolution paths

[jordanlewis]

See individual commits. This PR shaves 3% of the total number of allocations in the FlowSetup benchmark via a variety of methods.

name                                          old time/op    new time/op    delta
FlowSetup/vectorize=true/distribute=false-24     144µs ± 1%     142µs ± 2%  -1.01%  (p=0.023 n=10+10)

name                                          old alloc/op   new alloc/op   delta
FlowSetup/vectorize=true/distribute=false-24    28.3kB ± 1%    27.8kB ± 1%  -1.76%  (p=0.000 n=9+9)

name                                          old allocs/op  new allocs/op  delta
FlowSetup/vectorize=true/distribute=false-24       232 ± 0%       224 ± 0%  -3.45%  (p=0.000 n=8+8)

[cockroach-teamcity]

This change is 

[yuzefovich]

nit: not sure where "accessors" in the fifth commit's title comes from.

Reviewed 1 of 1 files at r1, 4 of 4 files at r2, 1 of 1 files at r3, 1 of 1 files at r4, 1 of 1 files at r5.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @jordanlewis)

---

pkg/sql/execinfra/processorsbase.go, line 867 at r3 (raw file):

	pb.SemaCtx.TypeResolver = resolver

	return pb.Out.Init(post, coreOutputTypes, &pb.SemaCtx, pb.EvalCtx, output)

I don't understand the third commit - where is the allocation removed from?

[jordanlewis]

Done.

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @yuzefovich)

---

pkg/sql/execinfra/processorsbase.go, line 867 at r3 (raw file):

Previously, yuzefovich wrote…

I don't understand the third commit - where is the allocation removed from?

The compiler detects that the SemaCtx argument to Init must be heap-allocated. Before, we constructed a new SemaCtx - seemingly on the stack. But because Init needed a heap-allocated SemaCtx, the compiler promotes the SemaCtx to the heap: the object escapes.

Now, we add space on ProcessorBase for a SemaCtx. At this point, ProcessorBase is already on the heap. So, the compiler can directly use the address of ProcessorBase.SemaCtx safely - no allocations needed.

[yuzefovich]

Reviewed 5 of 5 files at r6, 4 of 4 files at r7, 1 of 1 files at r8, 1 of 1 files at r9, 1 of 1 files at r10.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @jordanlewis)

---

pkg/sql/execinfra/processorsbase.go, line 867 at r3 (raw file):

Previously, jordanlewis (Jordan Lewis) wrote…

The compiler detects that the SemaCtx argument to Init must be heap-allocated. Before, we constructed a new SemaCtx - seemingly on the stack. But because Init needed a heap-allocated SemaCtx, the compiler promotes the SemaCtx to the heap: the object escapes.

Now, we add space on ProcessorBase for a SemaCtx. At this point, ProcessorBase is already on the heap. So, the compiler can directly use the address of ProcessorBase.SemaCtx safely - no allocations needed.

Got it, thanks.

[jordanlewis]

bors r+

Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @jordanlewis)

[craig]

Build succeeded:

• GitHub CI (Cockroach)

[RaduBerinde]

@jordanlewis @lucy-zhang I have a comment related to the "remove pointless log allocs on the hot path" commit. The very long log line changed by that commit adds the full descriptor information to recorded traces, which makes SHOW TRACE FOR SESSION pretty much unusable (see below). Could we backport this commit to 20.2.x and 20.1.x (if the log is there)?

[jordanlewis]

Hmm... I'm thinking that we actually didn't mean to remove that from the trace. I think I should have used ExpensiveLogEnabled(ctx,2) instead of V(2)...

[yuzefovich]

Hm, I made a couple of changes like that too, didn't realize if log.V(n) would start hiding the log message from the trace.

[RaduBerinde]

Well, as I said, I think we should reconsider dumping that long info in each trace. Or at least split it into multiple logs.