| From: | Heikki Linnakangas <hlinnaka(at)iki(dot)fi> |
|---|---|
| To: | Tomas Vondra <tomas(at)vondra(dot)me>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>, Robert Haas <robertmhaas(at)gmail(dot)com>, Rahila Syed <rahilasyed90(at)gmail(dot)com> |
| Subject: | Re: Shared hash table allocations |
| Date: | 2026-03-30 15:28:58 |
| Message-ID: | 3026ec05-f664-4ebe-8bf6-0a1218b234ec@iki.fi |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
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
| Attachment | Content-Type | Size |
|---|---|---|
| v2-0001-Use-ShmemInitStruct-to-allocate-shmem-for-semapho.patch | text/x-patch | 1.8 KB |
| v2-0002-Remove-HASH_SEGMENT-option.patch | text/x-patch | 9.7 KB |
| v2-0003-Change-the-signature-of-dynahash-s-alloc-function.patch | text/x-patch | 8.4 KB |
| v2-0004-Merge-init-and-max-size-options-on-shmem-hash-tab.patch | text/x-patch | 10.2 KB |
| v2-0005-Prevent-shared-memory-hash-tables-from-growing-be.patch | text/x-patch | 5.1 KB |
| v2-0006-Allocate-all-parts-of-shmem-hash-table-from-a-sin.patch | text/x-patch | 11.9 KB |
| v2-0007-Remove-HASH_DIRSIZE-always-use-the-default-algori.patch | text/x-patch | 6.9 KB |
| v2-0008-Make-ShmemIndex-visible-in-the-pg_shmem_allocatio.patch | text/x-patch | 2.1 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Sami Imseih | 2026-03-30 15:32:41 | Re: Add pg_stat_autovacuum_priority |
| Previous Message | Matthias van de Meent | 2026-03-30 15:26:35 | Re: [BUG] Excessive memory usage with update on STORED generated columns. |