Subject: Re: Shared memory size computation oversight?
To: Julien Rouhaud <rjuju123@gmail.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>,
 Robert Haas <robertmhaas@gmail.com>
References: <20210227080815.jryyhy6lrn6gahkm@nol>
 <da2a9fd1-b135-5dc4-ff5d-3e3447a6b3c9@enterprisedb.com>
 <CAOBaU_ZpZdNHb4w8AAMttns3U8hUEsnFtDrdUvuaGUKBvHDcUw@mail.gmail.com>
From: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
Message-ID: <ecf7aa59-680e-3910-ae04-d25925b0e00d@enterprisedb.com>
Date: Fri, 13 Aug 2021 10:52:50 +0200
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:78.0)
 Gecko/20100101 Thunderbird/78.12.0
MIME-Version: 1.0
In-Reply-To: 
 <CAOBaU_ZpZdNHb4w8AAMttns3U8hUEsnFtDrdUvuaGUKBvHDcUw@mail.gmail.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Archived-At: 
 <https://www.postgresql.org/message-id/ecf7aa59-680e-3910-ae04-d25925b0e00d%40enterprisedb.com>
Precedence: bulk

On 12.08.21 16:18, Julien Rouhaud wrote:
> 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.

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.