Re: Shared memory size computation oversight?

From: Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com>
To: Julien Rouhaud <rjuju123(at)gmail(dot)com>
Cc: PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Robert Haas <robertmhaas(at)gmail(dot)com>
Subject: Re: Shared memory size computation oversight?
Date: 2021-08-13 08:52:50
Message-ID: ecf7aa59-680e-3910-ae04-d25925b0e00d@enterprisedb.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 12.08.21 16:18, Julien Rouhaud wrote:
> On Thu, Aug 12, 2021 at 9:34 PM Peter Eisentraut
> <peter(dot)eisentraut(at)enterprisedb(dot)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.

I think you can do different things here to create inconsistent results,
but I think there should be one common, standard, normal,
straightforward way to get a correct result.

>> 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.

Yeah, in that case it's probably rather the case that there is one
CACHELINEALIGN() too few, since pg_stat_statements does two separate
shmem allocations.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Kapila 2021-08-13 09:08:37 Re: pgsql: pgstat: Bring up pgstat in BaseInit() to fix uninitialized use o
Previous Message Gavin Flower 2021-08-13 07:33:25 Re: Default to TIMESTAMP WITH TIME ZONE?