| 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/
>
>
>
>
>
>
>
| 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() |