Remove unused fields from BufferCacheNumaRec
Hi hackers,
While working on [1]/messages/by-id/aR9I29QgGdyUMkJq@ip-10-97-1-34.eu-west-3.compute.internal, I noticed that there are unused fields in BufferCacheNumaRec
since ba2a3c2302f.
Also, I noticed that a comment was not at the correct location in
pg_buffercache_numa_pages().
The attached takes care of both.
[1]: /messages/by-id/aR9I29QgGdyUMkJq@ip-10-97-1-34.eu-west-3.compute.internal
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
Attachments:
v1-0001-Remove-unused-fields-from-BufferCacheNumaRec.patchtext/x-diff; charset=us-asciiDownload
From 6dcc015e70ed18f22f1de18372910ac603fd2f0c Mon Sep 17 00:00:00 2001
From: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Date: Fri, 21 Nov 2025 05:53:50 +0000
Subject: [PATCH v1] Remove unused fields from BufferCacheNumaRec
These were added by ba2a3c2302f and never been used.
Also move (and re-word a bit) a comment that was not at the right place.
---
contrib/pg_buffercache/pg_buffercache_pages.c | 13 +++++--------
1 file changed, 5 insertions(+), 8 deletions(-)
100.0% contrib/pg_buffercache/
diff --git a/contrib/pg_buffercache/pg_buffercache_pages.c b/contrib/pg_buffercache/pg_buffercache_pages.c
index c29b784dfa1..1fe350783b1 100644
--- a/contrib/pg_buffercache/pg_buffercache_pages.c
+++ b/contrib/pg_buffercache/pg_buffercache_pages.c
@@ -82,9 +82,6 @@ typedef struct
typedef struct
{
TupleDesc tupdesc;
- int buffers_per_page;
- int pages_per_buffer;
- int os_page_size;
BufferCacheNumaRec *record;
} BufferCacheNumaContext;
@@ -368,7 +365,11 @@ pg_buffercache_numa_pages(PG_FUNCTION_ARGS)
os_page_ptrs = palloc0(sizeof(void *) * os_page_count);
os_page_status = palloc(sizeof(uint64) * os_page_count);
- /* Fill pointers for all the memory pages. */
+ /*
+ * Fill pointers for all the memory pages. This loop stores into
+ * os_page_ptrs[] and touches (if needed) addresses as input to one
+ * big move_pages(2) inquiry system call.
+ */
idx = 0;
for (char *ptr = startptr; ptr < endptr; ptr += os_page_size)
{
@@ -449,10 +450,6 @@ pg_buffercache_numa_pages(PG_FUNCTION_ARGS)
* We don't hold the partition locks, so we don't get a consistent
* snapshot across all buffers, but we do grab the buffer header
* locks, so the information of each buffer is self-consistent.
- *
- * This loop touches and stores addresses into os_page_ptrs[] as input
- * to one big move_pages(2) inquiry system call. Basically we ask for
- * all memory pages for NBuffers.
*/
startptr = (char *) TYPEALIGN_DOWN(os_page_size, (char *) BufferGetBlock(1));
idx = 0;
--
2.34.1
On Fri, Nov 21, 2025 at 11:34:01AM +0000, Bertrand Drouvot wrote:
While working on [1], I noticed that there are unused fields in BufferCacheNumaRec
since ba2a3c2302f.Also, I noticed that a comment was not at the correct location in
pg_buffercache_numa_pages().The attached takes care of both.
[1]: /messages/by-id/aR9I29QgGdyUMkJq@ip-10-97-1-34.eu-west-3.compute.internal
It seems to me that you are right here. Will appl, thanks.
--
Michael
On Nov 21, 2025, at 19:34, Bertrand Drouvot <bertranddrouvot.pg@gmail.com> wrote:
Hi hackers,
While working on [1], I noticed that there are unused fields in BufferCacheNumaRec
since ba2a3c2302f.Also, I noticed that a comment was not at the correct location in
pg_buffercache_numa_pages().The attached takes care of both.
As long as compile passes, that proves the removal of the unused fields is safe. And by reading the code, I believe the movement of the comment is also correct.
Looks like you have done a little bit rewording on the comment:
1. "This loop stores into os_page_ptrs[]” is understandable, but feels a bit incomplete to me as non-English-speaking. I understand your intention is to make “stores” and “touches” to share “addresses”. But this is not a strong opinion comment, if Michael considers okay, I will be fine as well.
2. Instead of saying “if needed”, why don’t explicitly mention something like “on the first pass”.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
On Sun, Nov 23, 2025 at 12:27:08PM +0800, Chao Li wrote:
1. "This loop stores into os_page_ptrs[]” is understandable, but
feels a bit incomplete to me as non-English-speaking. I understand
your intention is to make “stores” and “touches” to share
“addresses”. But this is not a strong opinion comment, if Michael
considers okay, I will be fine as well.
I have actually used something slightly different at the end, and
appl-ed the result.
--
Michael
Hi,
Thanks for having looked at it!
On Sun, Nov 23, 2025 at 12:27:08PM +0800, Chao Li wrote:
As long as compile passes, that proves the removal of the unused fields is safe.
Most of the time, but not always, see [1]/messages/by-id/aS2b3LoUypW1/Gdz@ip-10-97-1-34.eu-west-3.compute.internal.
[1]: /messages/by-id/aS2b3LoUypW1/Gdz@ip-10-97-1-34.eu-west-3.compute.internal
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com