On Thu, Aug 12, 2021 at 9:34 PM Peter Eisentraut
<peter.eisentraut@enterprisedb.com> wrote:
>
> On 27.02.21 09:08, Julien Rouhaud wrote:
> > PFA a patch that fixes pg_prewarm and pg_stat_statements explicit alignment to
> > CACHELINEALIGN, and also update the alignment in hash_estimate_size() to what I
> > think ShmemInitHash will actually consume.
>
> For extensions, wouldn't it make things better if
> RequestAddinShmemSpace() applied CACHELINEALIGN() to its argument?
>
> If you consider the typical sequence of RequestAddinShmemSpace(mysize())
> and later ShmemInitStruct(..., mysize(), ...),
But changing RequestAddinShmemSpace() to apply CACHELINEALIGN() would
only really work for that specific usage only? If an extension does
multiple allocations you can't rely on correct results.
> then the size gets
> rounded up to cache-line size in the second case and not the first. The
> premise in your patch is that the extension should figure that out
> before calling RequestAddinShmemSpace(), but that seems wrong.
>
> Btw., I think your patch was wrong to apply CACHELINEALIGN() to
> intermediate results rather than at the end.
I'm not sure why you think it's wrong. It's ShmemInitStruct() that
will apply the padding, so if the extension calls it multiple times
(whether directly or indirectly), then the padding will be added
multiple times. Which means that in theory the extension should
account for it multiple time in the amount of memory it's requesting.
But given the later discussion, ISTM that there's an agreement that
any number of CACHELINEALIGN() overhead for any number of allocation
can be ignored as it should be safely absorbed by the 100kB extra
space. I think that the real problem, as mentioned in my last email,
is that the shared htab size estimation doesn't really scale and can
easily eat the whole extra space if you use some not that unrealistic
parameters. I still don't know if I made any mistake trying to
properly account for the htab allocation, but if I didn't it can be
problematic.
