Hi,
On Fri, Apr 04, 2025 at 09:25:57PM +0200, Tomas Vondra wrote:
> OK,
>
> here's v25 after going through the patches once more, fixing the issues
> mentioned by Bertrand, etc.
Thanks!
> I think 0001 and 0002 are fine,
Agree, I just have some cosmetic nits comments: please find them in
nit-bertrand-0002.txt attached.
> I have a
> couple minor questions about 0003.
>
> - I was wondering if maybe we should have some "global ID" of memory
> page, so that with large memory pages it's indicated the buffers are on
> the same memory page. Right now each buffer starts page_num from 0, but
> it should not be very hard to have a global counter. Opinions?
I think that's a good idea. We could then add a new column (say os_page_id) that
would help identify which buffers are sharing the same "physical" page.
> 0003
> ----
> - Minor formatting tweaks, comment improvements.
> - Isn't this comment a bit confusing / misleading?
>
> /* Get number of OS aligned pages */
>
> AFAICS the point is to adjust the allocated_size to be a multiple of
> os-page_size, to get "all" memory pages the segment uses. But that's not
> what I understand by "aligned page" (which is about there the page is
> expected to start).
Agree, what about?
"
Align the start of the allocated size to an OS page size boundary and then get
the total number of OS pages used by this segment"
"
> - There's a comment at the end which talks about "ignored segments".
> IMHO that type of information should be in the function comment,
> but I'm
> also not quite sure I understand what "output shared memory" is ...
I think that comes from the comments that are already in
pg_get_shmem_allocations().
I think that those are located here and worded that way to ease to understand
what is not in with pg_get_shmem_allocations_numa() if one look at both
functions. That said, I'm +1 to put this kind of comments in the function comment.
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
