Re: Use PGAlignedBlock instead of "char buf[BLCKSZ]" in more places

From: Peter Eisentraut <peter(at)eisentraut(dot)org>
To: Michael Paquier <michael(at)paquier(dot)xyz>, Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>
Cc: PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Use PGAlignedBlock instead of "char buf[BLCKSZ]" in more places
Date: 2023-12-04 14:53:44
Message-ID: 71d26836-c03b-4f1c-9f23-e81370f03ac1@eisentraut.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 04.12.23 06:46, Michael Paquier wrote:
> On Mon, Dec 04, 2023 at 06:59:13AM +0530, Bharath Rupireddy wrote:
>> The commit 44cac934 replaced "char buf[BLCKSZ]" with PGAlignedBlock to
>> avoid issues on alignment-picky hardware. While it replaced most of the
>> instances, there are still some more left. How about we use PGAlignedBlock
>> there too, something like the attached patch? A note [2] in the commit
>> 44cac934 says that ensuring proper alignment makes kernel data transfers
>> fasters and the left-over "char buf[BLCKSZ]" either do read() or write()
>> system calls, so it might be worth to align them with PGAlignedBlock.
>>
>> Thoughts?
>
> The buffers used to write the lock file and the TLI history file are
> not page buffers, and this could make code readers think that these
> are pages.

The type is called "aligned block", not "aligned buffer" or "aligned
page", so I don't think it's incorrect to try to use it.

So I am honestly not sure if there's a point in changing
> them because the current code is not incorrect, isn't it? It looks
> like 2042b3428d39 for the TLI history file and 52948169bcdd for the
> lock file began using BLCKSZ because that was just a handy thing to
> do, and because we know they would never get beyond that.

Yeah, it's not clear why these need to be block-sized. We shouldn't
perpetuate this without more clarity about this.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Alexander Lakhin 2023-12-04 15:00:00 Re: PATCH: Add REINDEX tag to event triggers
Previous Message Matthias van de Meent 2023-12-04 14:41:24 Re: Avoid detoast overhead when possible