Re: Fix unnecessary shared memory page allocation in CalculateShmemSize()

From: Osama Abdul Qader <osamaabdulqader(dot)cs(at)gmail(dot)com>
To: Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com>
Cc: Matthias van de Meent <boekewurm+postgres(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Fix unnecessary shared memory page allocation in CalculateShmemSize()
Date: 2026-09-14 09:59:51
Message-ID: CAC+8b5hWbk8BxYW-8MyoV-24EwFALoQSoZE06q_6yVKyVsDjPA@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Greetings of the day,

I was able to reproduce the bug locally and have applied both *TYPEALIGN(8192,
size) *and the patch that Chao attached in his email.

I have tested both approaches locally.

*TYPEALIGN(8192, size) *does handle the rounding more concisely, and I
confirmed that it leaves an already 8192-byte-aligned value unchanged.

The proposed conditional *add_size() *change also fixes the issue, while
retaining the overflow protection provided by *add_size()*.

So I think the main question is whether overflow protection is important
here. If it is, I would prefer the proposed conditional change; otherwise,
*TYPEALIGN()* seems cleaner.

With regards,
Osama Abdul Qader

On Mon, Sep 14, 2026 at 2:48 PM Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> wrote:

>
>
> > On Sep 14, 2026, at 16:14, Matthias van de Meent <
> boekewurm+postgres(at)gmail(dot)com> wrote:
> >
> > On Mon, 14 Sept 2026 at 09:19, Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> wrote:
> >>
> >> Hi,
> >>
> >> I just noticed this item when I went through my TODO list today. I
> remember finding this issue a few months ago, but at that time, only bugs
> new to PG19 were being processed, so I put it on my TODO list.
> >>
> >> This is a small issue, but it has been there for many years.
> CalculateShmemSize() has logic to round size to a multiple of a typical
> page size:
> >> ```
> >> /* might as well round it off to a multiple of a typical page
> size */
> >> size = add_size(size, 8192 - (size % 8192));
> >> ```
> >>
> >> When size is already a multiple of 8192, this add_size() call is not
> needed; it only results in an extra 8192 bytes being allocated in shared
> memory. The fix is simple:
> >> ```
> >> if (size % 8192 != 0)
> >> /* might as well round it off to a multiple of a typical
> page size */
> >> size = add_size(size, 8192 - (size % 8192));
> >> ```
> >>
> >> I put the comment within the if clause because I remember Tom once
> mentioning that this would be the preferred style.
> >
> > Shouldn't a TYPEALIGN(8192, size) do the trick here, and do it more
> > concise and better?
> >
>
> TYPEALIGN(8192, size) would handle rounding more concisely, but I see
> add_size() has an overflow protection, maybe that matters?
>
> Best regards,
> --
> Chao Li (Evan)
> HighGo Software Co., Ltd.
> https://www.highgo.com/
>
>
>
>
>
>
>

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Jan Nidzwetzki 2026-09-14 10:13:46 Re: [PATCH] Speed up repeat() for larger counts
Previous Message Chao Li 2026-09-14 09:18:06 Re: Fix unnecessary shared memory page allocation in CalculateShmemSize()