Nathan Bossart <nathandbossart@gmail.com> wrote:
> On Wed, Jul 10, 2024 at 05:00:13PM +0200, Antonin Houska wrote:
> > I don't quite understand why TransactionIdIsCurrentTransactionId() implements
> > binary search in ParallelCurrentXids "from scratch" instead of using
> > bsearch().
>
> I believe there are a number of open-coded binary searches in the tree.
Not sure if there are many, but I could find some:
* TransactionIdIsCurrentTransactionId()
* RelationHasSysCache()
* pg_dump.c:getRoleName()
> My concern with switching them to bsearch() would be the performance impact
> of using a function pointer for the comparisons. Perhaps we could add a
> couple of inlined binary search implementations for common types to replace
> many of the open-coded ones.
bsearch() appears to be used widely, but o.k., I don't insist on using it to
replace the existing open-coded searches.
What actually bothers me more than the absence of bsearch() is that
TransactionIdIsCurrentTransactionId() implements the binary search from
scratch. Even w/o bsearch(), it can still call TransactionIdInArray(). I ran
into the problem when working on [1], which adds one more XID array.
Does the attached patch seem worth being applied separately, or at all?
[1] https://www.postgresql.org/message-id/82651.1720540558%40antos
--
Antonin Houska
Web: https://www.cybertec-postgresql.com
