From 9cf33cb6c80e867651681216d682ae2505e0e954 Mon Sep 17 00:00:00 2001 From: Thomas Munro Date: Sat, 15 Feb 2025 14:47:25 +1300 Subject: [PATCH v2.4 03/29] Refactor read_stream.c's circular arithmetic. Several places have open-coded circular index arithmetic. Make some common functions for better readability and consistent assertion checking. This avoids adding yet more open-coded duplication in later patches, and standardizes on the vocabulary "advance" and "retreat" as used elsewhere in PostgreSQL. --- src/backend/storage/aio/read_stream.c | 78 +++++++++++++++++++++------ 1 file changed, 61 insertions(+), 17 deletions(-) diff --git a/src/backend/storage/aio/read_stream.c b/src/backend/storage/aio/read_stream.c index 99e44ed99fe..1c93fcae19b 100644 --- a/src/backend/storage/aio/read_stream.c +++ b/src/backend/storage/aio/read_stream.c @@ -224,6 +224,55 @@ read_stream_unget_block(ReadStream *stream, BlockNumber blocknum) stream->buffered_blocknum = blocknum; } +/* + * Increment index, wrapping around at queue size. + */ +static inline void +read_stream_index_advance(ReadStream *stream, int16 *index) +{ + Assert(*index >= 0); + Assert(*index < stream->queue_size); + + *index += 1; + if (*index == stream->queue_size) + *index = 0; +} + +/* + * Increment index by n, wrapping around at queue size. + */ +static inline void +read_stream_index_advance_n(ReadStream *stream, int16 *index, int16 n) +{ + Assert(*index >= 0); + Assert(*index < stream->queue_size); + Assert(n <= MAX_IO_COMBINE_LIMIT); + + *index += n; + if (*index >= stream->queue_size) + *index -= stream->queue_size; + + Assert(*index >= 0); + Assert(*index < stream->queue_size); +} + +#if defined(CLOBBER_FREED_MEMORY) || defined(USE_VALGRIND) +/* + * Decrement index, wrapping around at queue size. + */ +static inline void +read_stream_index_retreat(ReadStream *stream, int16 *index) +{ + Assert(*index >= 0); + Assert(*index < stream->queue_size); + + if (*index == 0) + *index = stream->queue_size - 1; + else + *index -= 1; +} +#endif + static void read_stream_start_pending_read(ReadStream *stream, bool suppress_advice) { @@ -302,11 +351,8 @@ read_stream_start_pending_read(ReadStream *stream, bool suppress_advice) &stream->buffers[stream->queue_size], sizeof(stream->buffers[0]) * overflow); - /* Compute location of start of next read, without using % operator. */ - buffer_index += nblocks; - if (buffer_index >= stream->queue_size) - buffer_index -= stream->queue_size; - Assert(buffer_index >= 0 && buffer_index < stream->queue_size); + /* Move to the location of start of next read. */ + read_stream_index_advance_n(stream, &buffer_index, nblocks); stream->next_buffer_index = buffer_index; /* Adjust the pending read to cover the remaining portion, if any. */ @@ -334,12 +380,12 @@ read_stream_look_ahead(ReadStream *stream, bool suppress_advice) /* * See which block the callback wants next in the stream. We need to * compute the index of the Nth block of the pending read including - * wrap-around, but we don't want to use the expensive % operator. + * wrap-around. */ - buffer_index = stream->next_buffer_index + stream->pending_read_nblocks; - if (buffer_index >= stream->queue_size) - buffer_index -= stream->queue_size; - Assert(buffer_index >= 0 && buffer_index < stream->queue_size); + buffer_index = stream->next_buffer_index; + read_stream_index_advance_n(stream, + &buffer_index, + stream->pending_read_nblocks); per_buffer_data = get_per_buffer_data(stream, buffer_index); blocknum = read_stream_get_block(stream, per_buffer_data); if (blocknum == InvalidBlockNumber) @@ -777,12 +823,12 @@ read_stream_next_buffer(ReadStream *stream, void **per_buffer_data) */ if (stream->per_buffer_data) { + int16 index; void *per_buffer_data; - per_buffer_data = get_per_buffer_data(stream, - oldest_buffer_index == 0 ? - stream->queue_size - 1 : - oldest_buffer_index - 1); + index = oldest_buffer_index; + read_stream_index_retreat(stream, &index); + per_buffer_data = get_per_buffer_data(stream, index); #if defined(CLOBBER_FREED_MEMORY) /* This also tells Valgrind the memory is "noaccess". */ @@ -800,9 +846,7 @@ read_stream_next_buffer(ReadStream *stream, void **per_buffer_data) stream->pinned_buffers--; /* Advance oldest buffer, with wrap-around. */ - stream->oldest_buffer_index++; - if (stream->oldest_buffer_index == stream->queue_size) - stream->oldest_buffer_index = 0; + read_stream_index_advance(stream, &stream->oldest_buffer_index); /* Prepare for the next call. */ read_stream_look_ahead(stream, false); -- 2.48.1.76.g4e746b1a31.dirty