Re: Optimize SnapBuildPurgeOlderTxn: use in-place compaction instead of temporary array

Xuneng Zhou <xunengzhou(at)gmail(dot)com> writes:
> On Thu, Jan 8, 2026 at 4:49 PM Neil Chen <carpenter(dot)nail(dot)cz(at)gmail(dot)com> wrote:
>> I’ve given this patch a cursory review, and it looks good overall.
>> Considering the impact of this change, should we add a regression test for it, or provide a unit test to ensure the correctness of the modification?
>> I think it would be more receptive to merging such a "subtle performance optimization" only when its correctness is fully guaranteed.

> Thanks for your review. I think we can add a tap test for it.

What makes you think this code isn't adequately tested already?
The coverage report at

https://coverage.postgresql.org/src/backend/replication/logical/snapbuild.c.gcov.html

shows SnapBuildPurgeOlderTxn as pretty fully exercised.

A more interesting question is whether it's worth bothering with
at all. This code only runs when we receive a running-xacts WAL
record, which is infrequent. So it seems pretty likely to me that
nobody could ever detect any performance difference from saving
a palloc/pfree here. Perhaps the patch is worth applying on the
grounds that it makes the function shorter and clearer, but that's
pretty much in the eye of the beholder.

On the whole, I think we've got bigger fish to fry.

regards, tom lane