Re: Shared memory size computation oversight?

On Thu, Mar 04, 2021 at 04:05:10PM +0900, Michael Paquier wrote:
> On Thu, Mar 04, 2021 at 12:40:52AM +0800, Julien Rouhaud wrote:
> > On Wed, Mar 03, 2021 at 11:23:54AM -0500, Tom Lane wrote:
> >> I have not looked at this patch, but I think the concern is basically that
> >> if we have space-estimation infrastructure that misestimates what it is
> >> supposed to estimate, then if that infrastructure is used to estimate the
> >> size of any of the "big hog" data structures, we might misestimate by
> >> enough that the slop factor wouldn't hide it.
> >
> > Exactly. And now that I looked deeper I can see that multiple estimates are
> > entirely ignoring the padding alignment (e.g. ProcGlobalShmemSize()), which can
> > exceed the 6kB originally estimated by Robert.
>
> We are going to have a hard time catching up all the places that are
> doing an incorrect estimation, and have an even harder time making
> sure that similar errors don't happen in the future. Should we add a
> {add,mul}_shmem_size() to make sure that the size calculated is
> correctly aligned, that uses CACHELINEALIGN before returning the
> result size?

I was also considering adding new (add|mull)_*_size functions to avoid having
too messy code. I'm not terribly happy with xxx_shm_size(), as not all call to
those functions would require an alignment. Maybe (add|mull)shmemalign_size?

But before modifying dozens of calls, should we really fix those or only
increase a bit the "slop factor", or a mix of it?

For instance, I can see that for instance BackendStatusShmemSize() never had
any padding consideration, while others do.

Maybe only fixing contribs, some macro like PredXactListDataSize that already
do a MAXALIGN, SimpleLruShmemSize and hash_estimate_size() would be a short
patch and should significantly improve the estimation.