fix: a profile must not measure the assert build or its own fixture (#445)

[jdatcmd]

Two artifacts in bench/run_profile.sh, found while profiling the load for #445. Both are the class that script already exists to prevent: output that looks like data.

1. An assert build is not a slower version of the same profile

--enable-cassert compiles in validation production never runs, and it does not distribute evenly. Profiled against the assert PG18 on this box, the top of the ingest profile was:

9.28%  postgres  verify_compact_attribute

The largest single self-time entry is a function that does not exist in a production build, with every other percentage diluted against it. A reader would reasonably have gone looking at tuple descriptor validation.

That is the same category as an unopenable perf event, which this script already refuses on the grounds that "profiling would report an empty profile as data". So it gets the same treatment: refuse, with PROFILE_ALLOW_CASSERT=1 to override, since an assert build is still worth profiling when it is the only one to hand and two assert builds compare fine with each other. The build is now printed with the event and the unwind method, for exactly the reason those are.

2. The ingest shape profiled its own data generator

INSERT INTO pw SELECT g, md5(g::text) FROM generate_series(1,200000) g

put md5_calc at 12.31% and pg_md5_hash at 1.65%. About 14% of an "ingest" measurement was the fixture inventing strings inside the timed statement, inflating the denominator every write-path entry was measured against. Rows are now materialised into a heap table first, outside the profiled statement, which is also closer to what a load is: rows arriving from somewhere rather than computed per row by the insert.

Proved by running it

• assert build now exits 1 with a FATAL naming the measured cost (checked the exit code, not the grep's)
• PROFILE_ALLOW_CASSERT=1 lets it through and labels every percentage, and the tail prints Build: ASSERT (--enable-cassert)
• on a non-assert build md5_calc leaves the profile entirely

Why this matters immediately

With both artifacts removed, the ingest profile is a different document, and the #472 result is far larger than I had reported:

	main	with #472 (#502)
encode_fsst_shared	43.22%	11.36%
PgColumnarFsstBuildChunkTable	19.22%	4.99%
fsst_count_add	4.66%	below cut
fsst_cand_cmp	2.91%	below cut
FSST total	~70%	~16%

I had posted 48% to 11.7% on #445 from measurements taken through both artifacts. Same direction, wrong size, and the corrected figures are on that issue.

#499's own 33.9% and 14.6% were taken the same way, so they are directionally right and quantitatively off. Noted in the commit rather than left to be rediscovered.

🤖 Generated with Claude Code

[ChronicallyJD]

Approving. Both are real, both are my defects, and the second one invalidates numbers I published.

The assert-build argument is the one I should have made myself. The script already refuses an unopenable perf event on the grounds that it "would report an empty profile as data", and then does not apply that same standard to the build it is profiling. verify_compact_attribute at 9.28% self time — the largest single entry, a function that does not exist in production, diluting every other percentage — is exactly the failure the rest of the script is built around. Refusing with an override, and printing the build alongside the event and the unwind method, is the consistent treatment.

The fixture defect is worse and it is mine. INSERT ... SELECT md5(g::text) FROM generate_series(...) put the data generator inside the timed statement, so about 14% of an "ingest" profile was the fixture inventing strings. I chose that shape for convenience and never asked what was in the denominator.

So #499's ingest figures are wrong and I have quoted them repeatedly — on #472 when sizing the cache, and in the benchmark report. Directionally right, quantitatively off, and the corrected ~70% makes the case for #502 stronger than the number I gave it. I will correct both.

Checked

• grep -c 'enable-cassert' does not false-positive on --disable-cassert: "disable" is d-i-s-a-b-l-e, so enable-cassert is not a substring of it. A build configured with an explicit --disable-cassert is correctly read as non-assert.
• grep -c prints 0 on no match and the || true absorbs its exit 1 under pipefail, so CASSERT is always a number and the != "0" test cannot fall through on an empty string.
• The override warns per run and the tail labels the build, so a saved profile carries its own caveat rather than depending on someone remembering.

One residual, and one new interaction

The heap source is now in the denominator. Reading 200,000 rows from pw_src is far cheaper than computing md5 per row, and "rows arriving from somewhere" is the right model, so this is a large improvement rather than a trade. Worth one clause in the comment that the source scan is still measured, so the next person does not rediscover it as a third artifact.

More interesting: once #502 lands, the ingest number stops being window-independent. pw is never truncated between loop iterations, so each pass appends another ~200,000 rows and the table grows for the whole window. That is harmless today. With the FSST verdict cache it is not: the first row groups pay for the decision and later ones reuse it, so the measured FSST share falls as the window lengthens, and PROFILE_SECS becomes an input to the answer.

That does not affect your before/after here — both arms ran the same window, which is why the comparison holds. It affects anyone who later profiles ingest at a different PROFILE_SECS and compares against these numbers.

Cheapest fix is a TRUNCATE pw inside the loop before each insert, so every iteration profiles a load starting from the same state; that also removes the growing-relation effect on row-group flushing. It does put a truncate in the timed statement, which is a smaller artifact than the one being removed but is still one, so the alternative is to leave it and say in the comment that the ingest shape measures a sustained load rather than a cold one, and that its FSST share is window-dependent once the cache exists.

Either is fine. Silently window-dependent is the thing to avoid, and it is not obvious from reading the script.

On the correction itself

Recording that #499's numbers were taken through both artifacts, in the commit rather than leaving it to be rediscovered, is the right call. I would rather have my published figures corrected in the open than quietly superseded, and the same applies to the 48% to 11.7% on #445.