bench: a profiling harness, and make two harnesses executable

[ChronicallyJD]

Adds bench/run_profile.sh, so a micro-optimisation can be aimed at a measured hot path rather than a guessed one, and sets the executable bit on two harnesses that were committed without it.

What it found on the first honest run

8M rows, PG 18.4, 6-second windows, serial:

shape	top of profile
decode (un-pushable LIKE, #426)	SB_MatchText 27.2% — core's matcher, not our decode
filtered (skippable range agg)	bitunpack 21.0%, PgColumnarReadNextRow 16.9%, ExecInterpExpr 13.8%
project (row-returning scan)	bitunpack 19.2%, PgColumnarReadNextRow 18.0%, ExecInterpExpr 13.4%
ingest (columnar write path, #445)	encode_fsst_shared 33.9%, PgColumnarFsstBuildChunkTable 14.6%, pg_qsort 5.5%, fsst_count_add 3.3%, fsst_cand_cmp 1.8%

About 59% of ingestion is FSST symbol-table construction. That is a measured answer to #445, where citus loads the same data faster, and it turns #472 (cache the keep/drop verdict instead of re-deciding per row group) from a plausible idea into a sized one.

On the read side, bitunpack is the largest single cost on both scan shapes while PgColumnarDecodeChunk itself is only 4–7%, so the inner bit-unpacking loop is where decode time actually sits — not where I would have guessed.

The guards are most of the script, and each one is a failure it produced

Every guard below exists because the harness did the thing it now prevents, and each failure produced output that looked like data:

• The perf event is probed before the fixture is built. This VM exposes no hardware PMU, so perf's default precise event cannot be opened, perf writes a zero-sized file, and perf report renders an empty profile without complaint. An empty profile reads as "nothing is hot" — after a 20M-row load.
• The unwind method is read from pg_config --cflags. The build has no -fno-omit-frame-pointer, so frame-pointer stacks truncate; it does keep .debug_info, so DWARF resolves. Both the event and the method print with the profile, since a percentage only compares against another sampled the same way.
• perf's stderr is never redirected. Discarding it is what turned the PMU diagnosis into an unexplained empty report.
• The sample count is asserted before any percentage is read.
• The backend is confirmed on-CPU and confirmed to be running this shape, via a marker unique to the shape and the run.
• The pid is asserted to differ from the previous shape's.

Why that last one exists

The first working version reported four clean, fully symbolized profiles that were all the same query. kill on the psql client does not stop the backend — the server keeps running its loop until it next writes to a client that is gone — so shape one's backend outlived its window and every later shape matched it on a generic '%LOOP%'.

Every guard passed. A backend was running, it was state=R, each profile had ~6,000 samples and a clean call tree. What caught it was a LIKE matcher appearing in an INSERT profile. That is luck, not a control, so it is now a control: the loops are time-bounded and expire on their own, the backend is terminated server-side, and two shapes sharing a pid is a hard failure that names why.

The executable bits

run_bench_join.sh and run_bench_readstream.sh are committed 0644. Invoked the way the other harnesses are they fail with Permission denied, which is how two of four were silently skipped in a full bench run. Mode-only change, no content.

Not included

No SUITES entry — bench/ is not registered anywhere, so this cannot conflict with an in-flight suite change.

[jdatcmd]

Good harness, and the findings are the kind that change what gets worked on: 59% of ingestion in FSST symbol-table construction sizes #472 and answers #445 with a measurement rather than a theory, and bitunpack dominating both scan shapes while PgColumnarDecodeChunk sits at 4 to 7% is genuinely not where I would have looked.

One change requested, and it is your own principle one level up.

The guards are real. I checked each against the script rather than the description

• perf event probed with perf stat before the fixture is built, with the fallback and a FATAL if neither opens
• evprobe.err captures perf's stderr instead of discarding it
• unwind method taken from pg_config --cflags, dwarf,8192 when frame pointers are absent, and both event and method printed with the profile
• marker is PROFILEMARK_${name}_$$, unique per shape and per invocation
• backend confirmed state = 'active' matching that marker and ps state R*
• pid = PREV_PID is a hard stop
• the DO block carries its own clock_timestamp() deadline, so a loop expires even if every cleanup fails
• pg_terminate_backend on the backend, waited on, before the client kill
• sample count asserted at 200 before any percentage is printed

The pid-collision story is worth the space you gave it. Four fully symbolized profiles of the same query, every guard passing, caught only because a LIKE matcher turned up in an INSERT profile, is exactly the shape this repository keeps producing.

cb_guards.sh is also 0644 and you correctly left it alone: it is sourced by run_clickbench.sh:126, never invoked. I checked because a sweep that fixed all three would have been wrong.

The requested change: a failed run still exits 0

FAIL: pid ... already profiled and both SKIP paths return from profile_shape without setting any state, and nothing after the loop inspects anything. So a run in which every shape failed on pid collision prints its FAIL lines, then:

== profile complete ==

and exits 0.

That is the defect this script is otherwise built to prevent, in the one place it does not look: the summary line says complete and the exit code says success, while the thing that was supposed to be measured was not. It is also #447 exactly, which you fixed, and which #455 then had to fix again one layer down when exit 2 collided with something else. The lesson recorded from that pair was two signals rather than one, because a single one cannot be made collision-proof, and here there is currently zero.

Concretely:

• a FAILED=1 flag set by the pid-collision branch and by the too-few-samples branch
• exit 1 at the end when it is set, and a final line naming which shapes produced no usable profile
• unknown shape: $shape should set it too. A typo in PROFILE_SHAPES currently profiles nothing and exits 0, which is the same failure with an easier cause

Whether the too-few-samples SKIP should be fatal or merely non-zero is your call. It is a legitimate "raise PROFILE_SECS" condition rather than a defect, so I would accept it counting as not-success without being called FAIL.

Nothing else. bench/ is unregistered so there is no SUITES interaction, the mode changes are correct, and CI is green on all 11.

[jdatcmd]

Re-reviewed at 17c29fa2. Unchanged since my review, so the exit-status request stands.

One addition from the second pass, and it is a clean bill rather than another request. I was suspicious of the sample-count parse, because [ "$samples" -lt 200 ] on a non-numeric value is a bash error rather than a false condition, and a thousands separator in perf's output would send a low-sample run straight past the guard and into printing percentages. That would be the same class as everything else the script defends against.

Tested it rather than raising it as a worry. perf 7.0.12:

              SAMPLE events:        409  (62.9%)
              SAMPLE events:        409

awk '/SAMPLE events/ { print $3; exit }' yields 409. Plain integer, no separator, and the exit correctly takes the first of the two lines. The guard is sound as written.

So the exit status is the only thing outstanding.

[ChronicallyJD]

Fixed and pushed as 481e491.

You are right, and it is this script's own principle in the one place the script did not apply it. Every guard I wrote checks the measurement; none checked the report of the measurement.

Two counters rather than one, per the #447 and #455 lesson that a single status value cannot be made collision-proof:

0  every requested shape produced a usable profile
1  a shape FAILED: pid collision, no backend appeared, or an unknown shape name
2  nothing failed, but a shape produced too few samples to attribute

I took your call on the too-few-samples case: not called FAIL. It is a legitimate "raise PROFILE_SECS" condition rather than a defect, so it is counted as not-success and named in the summary without being reported as a fault in the code under test. An unknown shape does fail, since profiling nothing and exiting 0 is the same defect with an easier cause.

Proved by removal rather than asserted:

PROFILE_SHAPES=bogus   ->  == profile FAILED ==   no usable profile for: bogus    exit 1
PROFILE_SHAPES=decode  ->  == profile complete ==                                 exit 0

Thanks for checking cb_guards.sh before assuming the sweep should have caught all three modes — it is sourced by run_clickbench.sh:126 and never invoked, so making it executable would have been wrong. That is the kind of thing I would have got wrong in a blanket fix.

[jdatcmd]

481e491a addresses it, and the shape you chose is better than what I asked for.

I said whether the thin-sample case should be fatal was your call. Two counters with three exit codes is the right answer, and grounding it in #447 and #455 is exactly the reasoning that keeps it from being simplified away later:

0  every requested shape produced a usable profile
1  at least one shape FAILED (pid collision, or a shape name that does not exist)
2  no shape failed, but at least one produced too few samples to attribute

Three things I checked rather than assumed:

• == profile complete == is now reachable only when both lists are empty.
• "no backend running the marker appeared" was upgraded from SKIP to FAIL. That is right and it is a change I did not ask for: a shape that was requested and did not run is a failure, not an abstention.
• unknown shape sets the flag, so a typo in PROFILE_SHAPES can no longer profile nothing and exit 0.

One observation, deliberately not a request

exit 2 is the code #455 moved away from, and for a reason that half-applies here: bash itself exits 2 on a syntax error, so "INCOMPLETE" and "this script no longer parses" are the same number. If this is ever wired into a make target or a cron that branches on the status, those two become indistinguishable.

I am not asking you to change it, because you have satisfied what #455 actually concluded: two signals rather than one. The == profile INCOMPLETE == marker is the second, and a broken script cannot print it. Recording the thought so that whoever automates this reads the marker rather than the number, and so the choice reads as considered.

Approving. Thanks for the profile itself, which is already changing what gets worked on: #472 landed on the strength of it (#502) and took about 2.5x out of the ingest path.