On 28/03/2026 02:14, Tomas Vondra wrote:
> * 0002 - +1 to getting rid of HASH_SEGMENT, but I don't see the point of
> renaming DEF_SEGSIZE to HASH_SEGSIZE. Isn't that a bit unnecessary?
DEF_SEGSIZE stands for "default segsize", but after this commit it's not
merely the default, it's the same hard-coded constant for every hash
table. That's why it seems prudent to rename it.
> * 0003 - I'd probably rename CurrentDynaHashCxt to something that
> doesn't seem like a "global" variable, e.g. "dynahashCxt"
Renamed it to "hcxt", as that's what the corresponding field in HTAB is
called.
> * 0004 - seems fine, +1 to get rid of unused pieces
To be clear, the init_size/max_size are not completely unused at the
moment: the lock manager sets max_size to 2 * init_size, and
wait_event.c used constants 16 and 128.
The point is that it doesn't give you a very wide range of scalability,
and I think it's better to not be flexible in that fashion. I would call
it sloppiness rather than flexibility.
> * 0005 - seems fine
>
> * 0006 - Doesn't this completely change the alignment? ShmemHashAlloc
> used to call ShmemAllocRaw, which is very careful to use CACHELINEALIGN.
> But now ShmemHashAlloc just does MAXALIGN, which ShmemAllocRaw claims is
> not enough on modern systems.
dynahash.c allocates multiple elements in each alloc() call, so even
though ShmemAllocRaw() returns a cacheline-aligned block, the individual
elements were not cacheline-aligned before this patch either. See
element_alloc() and choose_nelem_alloc().
> * 0007 - this left one comment referencing HASH_DIRSIZE in dynahash.c
Fixed
- Heikki
