Xlogprefetcher: Use atomic add for increment counter

Started by Ranier Vilela2 months ago5 messages
#1Ranier Vilela
ranier.vf@gmail.com
1 attachment(s)

Hi.

Use pg_atomic_fetch_add_u64 to replace pg_atomic_read_u64 and
pg_atomic_write_u64 calls.

This simplifies the logic and this increases the likelihood that the
operation will be successful.

patch attached.

best regards,
Ranier Vilela

Attachments:

use-atomic-add-for-increment-counter.patchapplication/octet-stream; name=use-atomic-add-for-increment-counter.patchDownload
diff --git a/src/backend/access/transam/xlogprefetcher.c b/src/backend/access/transam/xlogprefetcher.c
index ed3aacabc9..3dbd4925bd 100644
--- a/src/backend/access/transam/xlogprefetcher.c
+++ b/src/backend/access/transam/xlogprefetcher.c
@@ -351,7 +351,7 @@ static inline void
 XLogPrefetchIncrement(pg_atomic_uint64 *counter)
 {
 	Assert(AmStartupProcess() || !IsUnderPostmaster);
-	pg_atomic_write_u64(counter, pg_atomic_read_u64(counter) + 1);
+	pg_atomic_fetch_add_u64(counter, 1);
 }
 
 /*
#2Andres Freund
andres@anarazel.de
In reply to: Ranier Vilela (#1)
Re: Xlogprefetcher: Use atomic add for increment counter

Hi,

On 2025-11-07 11:28:06 -0300, Ranier Vilela wrote:

Use pg_atomic_fetch_add_u64 to replace pg_atomic_read_u64 and
pg_atomic_write_u64 calls.

This simplifies the logic and this increases the likelihood that the
operation will be successful.

How does it do so? As the assertions indicate, this can only be run from a
single process.

Greetings,

Andres Freund

#3Ranier Vilela
ranier.vf@gmail.com
In reply to: Andres Freund (#2)
Re: Xlogprefetcher: Use atomic add for increment counter

Em sex., 7 de nov. de 2025 às 11:41, Andres Freund <andres@anarazel.de>
escreveu:

Hi,

On 2025-11-07 11:28:06 -0300, Ranier Vilela wrote:

Use pg_atomic_fetch_add_u64 to replace pg_atomic_read_u64 and
pg_atomic_write_u64 calls.

This simplifies the logic and this increases the likelihood that the
operation will be successful.

How does it do so? As the assertions indicate, this can only be run from a
single process.

Can I rephrase that?

That simplifies the logic a bit.

best regards,
Ranier Vilela

#4Andres Freund
andres@anarazel.de
In reply to: Ranier Vilela (#3)
Re: Xlogprefetcher: Use atomic add for increment counter

On 2025-11-07 11:52:37 -0300, Ranier Vilela wrote:

Em sex., 7 de nov. de 2025 �s 11:41, Andres Freund <andres@anarazel.de>
escreveu:

On 2025-11-07 11:28:06 -0300, Ranier Vilela wrote:

Use pg_atomic_fetch_add_u64 to replace pg_atomic_read_u64 and
pg_atomic_write_u64 calls.

This simplifies the logic and this increases the likelihood that the
operation will be successful.

How does it do so? As the assertions indicate, this can only be run from a
single process.

Can I rephrase that?

That simplifies the logic a bit.

Maybe simpler, but also vastly slower than before. An atomic increment is
maybe two orders of magnitude more expensive than an unlocked read & write.

#5Ranier Vilela
ranier.vf@gmail.com
In reply to: Andres Freund (#4)
Re: Xlogprefetcher: Use atomic add for increment counter

Em sex., 7 de nov. de 2025 às 11:59, Andres Freund <andres@anarazel.de>
escreveu:

On 2025-11-07 11:52:37 -0300, Ranier Vilela wrote:

Em sex., 7 de nov. de 2025 às 11:41, Andres Freund <andres@anarazel.de>
escreveu:

On 2025-11-07 11:28:06 -0300, Ranier Vilela wrote:

Use pg_atomic_fetch_add_u64 to replace pg_atomic_read_u64 and
pg_atomic_write_u64 calls.

This simplifies the logic and this increases the likelihood that the
operation will be successful.

How does it do so? As the assertions indicate, this can only be run

from a

single process.

Can I rephrase that?

That simplifies the logic a bit.

Maybe simpler, but also vastly slower than before. An atomic increment is
maybe two orders of magnitude more expensive than an unlocked read & write.

Seriously, I didn't know.

It's best to withdraw the patch then.
Thanks for clarifying this.

best regards,
Ranier Vilela