Date: Tue, 9 Aug 2022 13:00:37 -0700
From: Nathan Bossart <nathandbossart@gmail.com>
To: John Naylor <john.naylor@enterprisedb.com>
Cc: Masahiko Sawada <sawada.mshk@gmail.com>,
	Andres Freund <andres@anarazel.de>,
	John Naylor <jcnaylor@gmail.com>,
	pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: optimize lookups in snapshot [sub]xip arrays
Message-ID: <20220809200037.GA1490626@nathanxps13>
References: 
 <CAFBsxsGmNefB-LAo8hNXLDAwWN0vHXc6H2A+NfFUQrj=rn50DQ@mail.gmail.com>
 <20220805202510.GA1208450@nathanxps13>
 <20220805220434.6h2aa7eww5rxird6@awork3.anarazel.de>
 <20220806181326.GA1216305@nathanxps13>
 <20220806212522.GA1221353@nathanxps13>
 <CAFBsxsFCtNAxbmSc9dc6xbF4HguBmMB7CDjm+8Z7uWEfTN+7oA@mail.gmail.com>
 <20220808223254.GA1393216@nathanxps13>
 <CAD21AoA_MEAENt9yaS5-rQ+_4yW4+GTYN16Jy0ntY+o8o7TtKg@mail.gmail.com>
 <20220809035129.GA1440829@nathanxps13>
 <CAFBsxsFsrucH5BiP092u0KiAcs73DJZoBR2yz1n7Sm91ZamGPg@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: 
 <CAFBsxsFsrucH5BiP092u0KiAcs73DJZoBR2yz1n7Sm91ZamGPg@mail.gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/20220809200037.GA1490626%40nathanxps13>
Precedence: bulk

On Tue, Aug 09, 2022 at 01:21:41PM +0700, John Naylor wrote:
> I decided I wasn't quite comfortable changing snapshot handling
> without further guarantees.  To this end, 0002 in the attached v11 is
> an addendum that adds assert checking (also pgindent and some
> comment-smithing). As I suspected, make check-world passes even with
> purposefully screwed-up coding. 0003 uses pg_lfind32 in syscache.c and
> I verified that sticking in the wrong answer will lead to a crash in
> assert-enabled builds in short order. I'd kind of like to throw this
> (or something else suitable) at the build farm first for that reason.
> It's simpler than the qsort/qunique/binary search that was there
> before, so that's nice, but I've not tried to test performance.

Your adjustments in 0002 seem reasonable to me.  I think it makes sense to
ensure there is test coverage for pg_lfind32(), but I don't know if that
syscache code is the right choice.  For non-USE_SSE2 builds, it might make
these lookups more expensive.  I'll look around to see if there are any
other suitable candidates.

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com