Re: Shared memory size computation oversight?

‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
On Thursday, March 4, 2021 9:21 AM, Michael Paquier <michael(at)paquier(dot)xyz> wrote:

> On Thu, Mar 04, 2021 at 03:23:33PM +0800, Julien Rouhaud wrote:
>
> > 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.
>
> The lack of complaints in this area looks to me like a sign that we
> may not really need to backpatch something, so I would not be against
> a precise chirurgy, with a separate set of {add,mul}_size() routines
> that are used where adapted, so as it is easy to track down which size
> estimations expect an extra padding. I would be curious to hear more
> thoughts from others here.
>
> Saying that, calling a new routine something like add_shmem_align_size
> makes it clear what's its purpose.

My limited opinion on the matter after spending some time yesterday through
the related code, is that with the current api it is easy to miss something
and underestimate or just be sloppy. If only from the readability point of
view, adding the proposed add_shmem_align_size will be beneficial.

I hold no opinion on backpatching.

//Georgios

>
> ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
>
> Michael