Re: Use simplehash.h instead of dynahash in SMgr

Hi,

On 2021-04-25 03:58:38 +1200, David Rowley wrote:
> Currently, we use dynahash hash tables to store the SMgrRelation so we
> can perform fast lookups by RelFileNodeBackend. However, I had in mind
> that a simplehash table might perform better. So I tried it...

> The test case was basically inserting 100 million rows one at a time
> into a hash partitioned table with 1000 partitions and 2 int columns
> and a primary key on one of those columns. It was about 12GB of WAL. I
> used a hash partitioned table in the hope to create a fairly
> random-looking SMgr hash table access pattern. Hopefully something
> similar to what might happen in the real world.

A potentially stupid question: Do we actually need to do smgr lookups in this
path? Afaict nearly all of the buffer lookups here will end up as cache hits in
shared buffers, correct?

Afaict we'll do two smgropens in a lot of paths:
1) XLogReadBufferExtended() does smgropen so it can do smgrnblocks()
2) ReadBufferWithoutRelcache() does an smgropen()

It's pretty sad that we constantly do two smgropen()s to start with. But in
the cache hit path we don't actually need an smgropen in either case afaict.

ReadBufferWithoutRelcache() does an smgropen, because that's
ReadBuffer_common()'s API. Which in turn has that API because it wants to use
RelationGetSmgr() when coming from ReadBufferExtended(). It doesn't seem
awful to allow smgr to be NULL and to pass in the rlocator in addition.

XLogReadBufferExtended() does an smgropen() so it can do smgrcreate() and
smgrnblocks(). But neither is needed in the cache hit case, I think. We could
do a "read-only" lookup in s_b, and only do the current logic in case that
fails?

Greetings,

Andres Freund