From a50597e6a0361475f31fc37522535d6ade4b9b0c Mon Sep 17 00:00:00 2001 From: Peter Eisentraut Date: Wed, 16 Sep 2026 09:38:13 +0200 Subject: [PATCH v3 2/5] Use C11 alignas instead of palloc/malloc for alignment Replace several cases where palloc()/malloc()/etc. was used solely to obtain an aligned buffer. Use alignas with a local variable instead, or one of the existing "AlignedBlock" types, where appropriate. The previous alignment guarantees are carried over. palloc-based allocations are replaced by alignas(MAXIMUM_ALIGNOF). Theoretically, malloc-based allocations should be replaced by alignas(max_align_t), but MSVC doesn't provide max_align_t, and so we use MAXIMUM_ALIGNOF here as well. They should be the same in practice. The allocations in InitWalRecovery() are not converted, because the comment says it is also this way to avoid wasting storage. The comment in XLogReaderAllocate(), on the other hand, was probably copied from InitWalRecovery(), but the part of the comment about wasting storage does not make sense in that context, so it is converted. The allocation in copy_file() is kept because we want to avoid putting large buffers on the stack and we might want that buffer to be even bigger sometime. Discussion: https://www.postgresql.org/message-id/flat/94a128da-bf3e-46bd-9e2d-609573c484da%40eisentraut.org --- src/backend/access/transam/xloginsert.c | 28 ++++++++--------------- src/backend/access/transam/xlogreader.c | 17 -------------- src/backend/access/transam/xlogrecovery.c | 8 +++---- src/backend/commands/sequence_xlog.c | 9 +++----- src/backend/storage/file/copydir.c | 6 ++++- src/backend/storage/ipc/dsm_impl.c | 10 ++++---- src/backend/storage/smgr/md.c | 6 ++--- src/bin/pg_resetwal/pg_resetwal.c | 7 ++---- src/include/access/xlogreader.h | 5 ++-- 9 files changed, 32 insertions(+), 64 deletions(-) diff --git a/src/backend/access/transam/xloginsert.c b/src/backend/access/transam/xloginsert.c index c9aff944a2e..70cbe9d709b 100644 --- a/src/backend/access/transam/xloginsert.c +++ b/src/backend/access/transam/xloginsert.c @@ -105,17 +105,6 @@ static uint64 mainrdata_len; /* total # of bytes in chain */ /* flags for the in-progress insertion */ static uint8 curinsert_flags = 0; -/* - * These are used to hold the record header while constructing a record. - * 'hdr_scratch' is not a plain variable, but is palloc'd at initialization, - * because we want it to be MAXALIGNed and padding bytes zeroed. - * - * For simplicity, it's allocated large enough to hold the headers for any - * WAL record. - */ -static XLogRecData hdr_rdt; -static char *hdr_scratch = NULL; - #define SizeOfXlogOrigin (sizeof(ReplOriginId) + sizeof(char)) #define SizeOfXLogTransactionId (sizeof(TransactionId) + sizeof(char)) @@ -622,6 +611,16 @@ XLogRecordAssemble(RmgrId rmid, uint8 info, XLogRecPtr *fpw_lsn, int *num_fpi, uint64 *fpi_bytes, bool *topxid_included) { + /* + * These are used to hold the record header while constructing a record. + * 'hdr_scratch' must be MAXALIGNed and padding bytes zeroed. + * + * For simplicity, it's allocated large enough to hold the headers for any + * WAL record. + */ + static XLogRecData hdr_rdt; + static alignas(MAXIMUM_ALIGNOF) char hdr_scratch[HEADER_SCRATCH_SIZE]; + XLogRecData *rdt; uint64 total_len = 0; int block_id; @@ -1430,11 +1429,4 @@ InitXLogInsert(void) sizeof(XLogRecData) * XLR_NORMAL_RDATAS); max_rdatas = XLR_NORMAL_RDATAS; } - - /* - * Allocate a buffer to hold the header information for a WAL record. - */ - if (hdr_scratch == NULL) - hdr_scratch = MemoryContextAllocZero(xloginsert_cxt, - HEADER_SCRATCH_SIZE); } diff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.c index 7db7c273b0c..3cd86dedc25 100644 --- a/src/backend/access/transam/xlogreader.c +++ b/src/backend/access/transam/xlogreader.c @@ -119,21 +119,6 @@ XLogReaderAllocate(int wal_segment_size, const char *waldir, /* initialize caller-provided support functions */ state->routine = *routine; - /* - * Permanently allocate readBuf. We do it this way, rather than just - * making a static array, for two reasons: (1) no need to waste the - * storage in most instantiations of the backend; (2) a static char array - * isn't guaranteed to have any particular alignment, whereas - * palloc_extended() will provide MAXALIGN'd storage. - */ - state->readBuf = (char *) palloc_extended(XLOG_BLCKSZ, - MCXT_ALLOC_NO_OOM); - if (!state->readBuf) - { - pfree(state); - return NULL; - } - /* Initialize segment info. */ WALOpenSegmentInit(&state->seg, &state->segcxt, wal_segment_size, waldir); @@ -145,7 +130,6 @@ XLogReaderAllocate(int wal_segment_size, const char *waldir, MCXT_ALLOC_NO_OOM); if (!state->errormsg_buf) { - pfree(state->readBuf); pfree(state); return NULL; } @@ -171,7 +155,6 @@ XLogReaderFree(XLogReaderState *state) pfree(state->errormsg_buf); if (state->readRecordBuf) pfree(state->readRecordBuf); - pfree(state->readBuf); pfree(state); } diff --git a/src/backend/access/transam/xlogrecovery.c b/src/backend/access/transam/xlogrecovery.c index fff8d57ac61..75082c9748c 100644 --- a/src/backend/access/transam/xlogrecovery.c +++ b/src/backend/access/transam/xlogrecovery.c @@ -532,10 +532,10 @@ InitWalRecovery(ControlFileData *ControlFile, bool *wasShutdown_ptr, /* * Allocate two page buffers dedicated to WAL consistency checks. We do - * it this way, rather than just making static arrays, for two reasons: - * (1) no need to waste the storage in most instantiations of the backend; - * (2) a static char array isn't guaranteed to have any particular - * alignment, whereas palloc() will provide MAXALIGN'd storage. + * it this way, rather than just making static arrays, because there is no + * need to waste the storage in most instantiations of the backend. + * (palloc() also gives us MAXALIGN'd storage, but nowadays that part + * could be handled with alignas.) */ replay_image_masked = (char *) palloc(BLCKSZ); primary_image_masked = (char *) palloc(BLCKSZ); diff --git a/src/backend/commands/sequence_xlog.c b/src/backend/commands/sequence_xlog.c index fcb3230cf3b..27e5e9b30db 100644 --- a/src/backend/commands/sequence_xlog.c +++ b/src/backend/commands/sequence_xlog.c @@ -26,7 +26,8 @@ seq_redo(XLogReaderState *record) uint8 info = XLogRecGetInfo(record) & ~XLR_INFO_MASK; Buffer buffer; Page page; - Page localpage; + PGAlignedBlock localpage_buf; + Page localpage = (Page) &localpage_buf; char *item; Size itemsz; xl_seq_rec *xlrec = (xl_seq_rec *) XLogRecGetData(record); @@ -44,10 +45,8 @@ seq_redo(XLogReaderState *record) * is examining the page concurrently; so we mustn't transiently trash the * buffer. The solution is to build the correct new page contents in * local workspace and then memcpy into the buffer. Then only bytes that - * are supposed to change will change, even transiently. We must palloc - * the local page for alignment reasons. + * are supposed to change will change, even transiently. */ - localpage = (Page) palloc(BufferGetPageSize(buffer)); PageInit(localpage, BufferGetPageSize(buffer), sizeof(sequence_magic)); sm = (sequence_magic *) PageGetSpecialPointer(localpage); @@ -65,8 +64,6 @@ seq_redo(XLogReaderState *record) MarkBufferDirty(buffer); XLogFlushBufferForRedoIfInit(record, 0, buffer); UnlockReleaseBuffer(buffer); - - pfree(localpage); } /* diff --git a/src/backend/storage/file/copydir.c b/src/backend/storage/file/copydir.c index ee42c796f77..fc0fe629855 100644 --- a/src/backend/storage/file/copydir.c +++ b/src/backend/storage/file/copydir.c @@ -155,7 +155,11 @@ copy_file(const char *fromfile, const char *tofile) #define FLUSH_DISTANCE (1024 * 1024) #endif - /* Use palloc to ensure we get a maxaligned buffer */ + /* + * Allocate the buffer dynamically, both because it is too large to put on + * the stack and because that gets us a maxaligned buffer. (For + * performance, we might want an even bigger buffer here.) + */ buffer = palloc(COPY_BUF_SIZE); /* diff --git a/src/backend/storage/ipc/dsm_impl.c b/src/backend/storage/ipc/dsm_impl.c index e8c07805f59..23869c1e7a3 100644 --- a/src/backend/storage/ipc/dsm_impl.c +++ b/src/backend/storage/ipc/dsm_impl.c @@ -867,13 +867,11 @@ dsm_impl_mmap(dsm_op op, dsm_handle handle, Size request_size, else { /* - * Allocate a buffer full of zeros. - * - * Note: palloc zbuffer, instead of just using a local char array, to - * ensure it is reasonably well-aligned; this may save a few cycles - * transferring data to the kernel. + * A buffer full of zeros. alignas ensures it is reasonably + * well-aligned, which may save a few cycles transferring it to the + * kernel. */ - char *zbuffer = (char *) palloc0(ZBUFFER_SIZE); + alignas(MAXIMUM_ALIGNOF) const char zbuffer[ZBUFFER_SIZE] = {0}; Size remaining = request_size; bool success = true; diff --git a/src/backend/storage/smgr/md.c b/src/backend/storage/smgr/md.c index 780c88c0630..d1334f188e1 100644 --- a/src/backend/storage/smgr/md.c +++ b/src/backend/storage/smgr/md.c @@ -1823,13 +1823,11 @@ _mdfd_getseg(SMgrRelation reln, ForkNumber forknum, BlockNumber blkno, */ if (nblocks < ((BlockNumber) RELSEG_SIZE)) { - char *zerobuf = palloc_aligned(BLCKSZ, PG_IO_ALIGN_SIZE, - MCXT_ALLOC_ZERO); + const PGIOAlignedBlock zerobuf = {0}; mdextend(reln, forknum, nextsegno * ((BlockNumber) RELSEG_SIZE) - 1, - zerobuf, skipFsync); - pfree(zerobuf); + &zerobuf, skipFsync); } flags = O_CREAT; } diff --git a/src/bin/pg_resetwal/pg_resetwal.c b/src/bin/pg_resetwal/pg_resetwal.c index ddaa23ac135..fda29eb1c1b 100644 --- a/src/bin/pg_resetwal/pg_resetwal.c +++ b/src/bin/pg_resetwal/pg_resetwal.c @@ -601,7 +601,7 @@ read_controlfile(void) { int fd; ssize_t len; - char *buffer; + alignas(MAXIMUM_ALIGNOF) char buffer[PG_CONTROL_FILE_SIZE]; pg_crc32c crc; if ((fd = open(XLOG_CONTROL_FILE, O_RDONLY | PG_BINARY, 0)) < 0) @@ -621,10 +621,7 @@ read_controlfile(void) exit(1); } - /* Use malloc to ensure we have a maxaligned buffer */ - buffer = (char *) pg_malloc(PG_CONTROL_FILE_SIZE); - - len = read(fd, buffer, PG_CONTROL_FILE_SIZE); + len = read(fd, buffer, sizeof buffer); if (len < 0) pg_fatal("could not read file \"%s\": %m", XLOG_CONTROL_FILE); close(fd); diff --git a/src/include/access/xlogreader.h b/src/include/access/xlogreader.h index 4a9a687e879..6e27b30fb35 100644 --- a/src/include/access/xlogreader.h +++ b/src/include/access/xlogreader.h @@ -260,10 +260,9 @@ struct XLogReaderState DecodedXLogRecord *decode_queue_tail; /* newest decoded record */ /* - * Buffer for currently read page (XLOG_BLCKSZ bytes, valid up to at least - * readLen bytes) + * Buffer for currently read page (valid up to at least readLen bytes) */ - char *readBuf; + alignas(MAXIMUM_ALIGNOF) char readBuf[XLOG_BLCKSZ]; uint32 readLen; /* last read XLOG position for data currently in readBuf */ -- 2.55.0