pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
remove pg_spin_delay() from atomics code
4+ messages / 2 participants
[nested] [flat]

* remove pg_spin_delay() from atomics code
@ 2026-05-05 17:52 Nathan Bossart <nathandbossart@gmail.com>
  2026-05-27 06:37 ` Re: remove pg_spin_delay() from atomics code Corey Huinker <corey.huinker@gmail.com>
  0 siblings, 1 reply; 4+ messages in thread

From: Nathan Bossart @ 2026-05-05 17:52 UTC (permalink / raw)
  To: pgsql-hackers

While inspecting what 32-bit optimizations we had, I noticed this line in
atomics/arch-x86.h:

    #elif defined(_MSC_VER) && defined(__x86_64__)

...which led me to rediscover what Thomas Munro had found a while back [0],
i.e., we aren't using the right x86_64 macro for MSVC in lots of places.
Upon further inspection, all of this pg_spin_delay() stuff seems to have
been unused since it was added in commit b64d92f1a5, so I'm guessing it's
just an implementation artifact that didn't get cleaned up.

Here's a patch to remove that dead code.

[0] https://postgr.es/m/CA%2BhUKGL8Hs-phHPugrWM%3D5dAkcT897rXyazYzLw-Szxnzgx-rA%40mail.gmail.com

-- 
nathan
From 588685e23134c1916ff9640b7c9eca155434e346 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@postgresql.org>
Date: Tue, 5 May 2026 12:37:50 -0500
Subject: [PATCH v1 1/1] remove pg_spin_delay()

---
 src/include/port/atomics.h          |  5 ---
 src/include/port/atomics/arch-x86.h | 54 -----------------------------
 src/include/port/atomics/generic.h  |  6 ----
 3 files changed, 65 deletions(-)

diff --git a/src/include/port/atomics.h b/src/include/port/atomics.h
index d8b1d20fe60..c50d95d29e2 100644
--- a/src/include/port/atomics.h
+++ b/src/include/port/atomics.h
@@ -154,11 +154,6 @@
 #define pg_read_barrier()	pg_read_barrier_impl()
 #define pg_write_barrier()	pg_write_barrier_impl()
 
-/*
- * Spinloop delay - Allow CPU to relax in busy loops
- */
-#define pg_spin_delay() pg_spin_delay_impl()
-
 /*
  * pg_atomic_init_flag - initialize atomic flag.
  *
diff --git a/src/include/port/atomics/arch-x86.h b/src/include/port/atomics/arch-x86.h
index bd6f4f56ca2..b317cf7a188 100644
--- a/src/include/port/atomics/arch-x86.h
+++ b/src/include/port/atomics/arch-x86.h
@@ -76,60 +76,6 @@ typedef struct pg_atomic_uint64
 } pg_atomic_uint64;
 #endif	/* __x86_64__ */
 
-#endif /* defined(__GNUC__) || defined(__INTEL_COMPILER) */
-
-#if !defined(PG_HAVE_SPIN_DELAY)
-/*
- * This sequence is equivalent to the PAUSE instruction ("rep" is
- * ignored by old IA32 processors if the following instruction is
- * not a string operation); the IA-32 Architecture Software
- * Developer's Manual, Vol. 3, Section 7.7.2 describes why using
- * PAUSE in the inner loop of a spin lock is necessary for good
- * performance:
- *
- *     The PAUSE instruction improves the performance of IA-32
- *     processors supporting Hyper-Threading Technology when
- *     executing spin-wait loops and other routines where one
- *     thread is accessing a shared lock or semaphore in a tight
- *     polling loop. When executing a spin-wait loop, the
- *     processor can suffer a severe performance penalty when
- *     exiting the loop because it detects a possible memory order
- *     violation and flushes the core processor's pipeline. The
- *     PAUSE instruction provides a hint to the processor that the
- *     code sequence is a spin-wait loop. The processor uses this
- *     hint to avoid the memory order violation and prevent the
- *     pipeline flush. In addition, the PAUSE instruction
- *     de-pipelines the spin-wait loop to prevent it from
- *     consuming execution resources excessively.
- */
-#if defined(__GNUC__) || defined(__INTEL_COMPILER)
-#define PG_HAVE_SPIN_DELAY
-static inline void
-pg_spin_delay_impl(void)
-{
-	__asm__ __volatile__(" rep; nop			\n");
-}
-#elif defined(_MSC_VER) && defined(__x86_64__)
-#define PG_HAVE_SPIN_DELAY
-static __forceinline void
-pg_spin_delay_impl(void)
-{
-	_mm_pause();
-}
-#elif defined(_MSC_VER)
-#define PG_HAVE_SPIN_DELAY
-static __forceinline void
-pg_spin_delay_impl(void)
-{
-	/* See comment for gcc code. Same code, MASM syntax */
-	__asm rep nop;
-}
-#endif
-#endif /* !defined(PG_HAVE_SPIN_DELAY) */
-
-
-#if defined(__GNUC__) || defined(__INTEL_COMPILER)
-
 #define PG_HAVE_ATOMIC_TEST_SET_FLAG
 static inline bool
 pg_atomic_test_set_flag_impl(volatile pg_atomic_flag *ptr)
diff --git a/src/include/port/atomics/generic.h b/src/include/port/atomics/generic.h
index fd64b6cbd86..daa772e9a6d 100644
--- a/src/include/port/atomics/generic.h
+++ b/src/include/port/atomics/generic.h
@@ -28,12 +28,6 @@
 #	define pg_write_barrier_impl pg_memory_barrier_impl
 #endif
 
-#ifndef PG_HAVE_SPIN_DELAY
-#define PG_HAVE_SPIN_DELAY
-#define pg_spin_delay_impl()	((void)0)
-#endif
-
-
 /* provide fallback */
 #if !defined(PG_HAVE_ATOMIC_FLAG_SUPPORT) && defined(PG_HAVE_ATOMIC_U32_SUPPORT)
 #define PG_HAVE_ATOMIC_FLAG_SUPPORT
-- 
2.50.1 (Apple Git-155)

Attachments:

  [text/plain] v1-0001-remove-pg_spin_delay.patch (3.7K, ../../afouZUH_eUkIj4i4@nathan/2-v1-0001-remove-pg_spin_delay.patch)
  download | inline diff:
From 588685e23134c1916ff9640b7c9eca155434e346 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@postgresql.org>
Date: Tue, 5 May 2026 12:37:50 -0500
Subject: [PATCH v1 1/1] remove pg_spin_delay()

---
 src/include/port/atomics.h          |  5 ---
 src/include/port/atomics/arch-x86.h | 54 -----------------------------
 src/include/port/atomics/generic.h  |  6 ----
 3 files changed, 65 deletions(-)

diff --git a/src/include/port/atomics.h b/src/include/port/atomics.h
index d8b1d20fe60..c50d95d29e2 100644
--- a/src/include/port/atomics.h
+++ b/src/include/port/atomics.h
@@ -154,11 +154,6 @@
 #define pg_read_barrier()	pg_read_barrier_impl()
 #define pg_write_barrier()	pg_write_barrier_impl()
 
-/*
- * Spinloop delay - Allow CPU to relax in busy loops
- */
-#define pg_spin_delay() pg_spin_delay_impl()
-
 /*
  * pg_atomic_init_flag - initialize atomic flag.
  *
diff --git a/src/include/port/atomics/arch-x86.h b/src/include/port/atomics/arch-x86.h
index bd6f4f56ca2..b317cf7a188 100644
--- a/src/include/port/atomics/arch-x86.h
+++ b/src/include/port/atomics/arch-x86.h
@@ -76,60 +76,6 @@ typedef struct pg_atomic_uint64
 } pg_atomic_uint64;
 #endif	/* __x86_64__ */
 
-#endif /* defined(__GNUC__) || defined(__INTEL_COMPILER) */
-
-#if !defined(PG_HAVE_SPIN_DELAY)
-/*
- * This sequence is equivalent to the PAUSE instruction ("rep" is
- * ignored by old IA32 processors if the following instruction is
- * not a string operation); the IA-32 Architecture Software
- * Developer's Manual, Vol. 3, Section 7.7.2 describes why using
- * PAUSE in the inner loop of a spin lock is necessary for good
- * performance:
- *
- *     The PAUSE instruction improves the performance of IA-32
- *     processors supporting Hyper-Threading Technology when
- *     executing spin-wait loops and other routines where one
- *     thread is accessing a shared lock or semaphore in a tight
- *     polling loop. When executing a spin-wait loop, the
- *     processor can suffer a severe performance penalty when
- *     exiting the loop because it detects a possible memory order
- *     violation and flushes the core processor's pipeline. The
- *     PAUSE instruction provides a hint to the processor that the
- *     code sequence is a spin-wait loop. The processor uses this
- *     hint to avoid the memory order violation and prevent the
- *     pipeline flush. In addition, the PAUSE instruction
- *     de-pipelines the spin-wait loop to prevent it from
- *     consuming execution resources excessively.
- */
-#if defined(__GNUC__) || defined(__INTEL_COMPILER)
-#define PG_HAVE_SPIN_DELAY
-static inline void
-pg_spin_delay_impl(void)
-{
-	__asm__ __volatile__(" rep; nop			\n");
-}
-#elif defined(_MSC_VER) && defined(__x86_64__)
-#define PG_HAVE_SPIN_DELAY
-static __forceinline void
-pg_spin_delay_impl(void)
-{
-	_mm_pause();
-}
-#elif defined(_MSC_VER)
-#define PG_HAVE_SPIN_DELAY
-static __forceinline void
-pg_spin_delay_impl(void)
-{
-	/* See comment for gcc code. Same code, MASM syntax */
-	__asm rep nop;
-}
-#endif
-#endif /* !defined(PG_HAVE_SPIN_DELAY) */
-
-
-#if defined(__GNUC__) || defined(__INTEL_COMPILER)
-
 #define PG_HAVE_ATOMIC_TEST_SET_FLAG
 static inline bool
 pg_atomic_test_set_flag_impl(volatile pg_atomic_flag *ptr)
diff --git a/src/include/port/atomics/generic.h b/src/include/port/atomics/generic.h
index fd64b6cbd86..daa772e9a6d 100644
--- a/src/include/port/atomics/generic.h
+++ b/src/include/port/atomics/generic.h
@@ -28,12 +28,6 @@
 #	define pg_write_barrier_impl pg_memory_barrier_impl
 #endif
 
-#ifndef PG_HAVE_SPIN_DELAY
-#define PG_HAVE_SPIN_DELAY
-#define pg_spin_delay_impl()	((void)0)
-#endif
-
-
 /* provide fallback */
 #if !defined(PG_HAVE_ATOMIC_FLAG_SUPPORT) && defined(PG_HAVE_ATOMIC_U32_SUPPORT)
 #define PG_HAVE_ATOMIC_FLAG_SUPPORT
-- 
2.50.1 (Apple Git-155)

^ permalink  raw  reply  [nested|flat] 4+ messages in thread

* Re: remove pg_spin_delay() from atomics code
  2026-05-05 17:52 remove pg_spin_delay() from atomics code Nathan Bossart <nathandbossart@gmail.com>
@ 2026-05-27 06:37 ` Corey Huinker <corey.huinker@gmail.com>
  2026-05-27 17:43   ` Re: remove pg_spin_delay() from atomics code Nathan Bossart <nathandbossart@gmail.com>
  0 siblings, 1 reply; 4+ messages in thread

From: Corey Huinker @ 2026-05-27 06:37 UTC (permalink / raw)
  To: Nathan Bossart <nathandbossart@gmail.com>; +Cc: pgsql-hackers

>
>
> Here's a patch to remove that dead code.


+1

The patch removes all 0 usages of pg_spin_delay, and all mentions of
pg_spin_delay.

All the associated comments were removed, no more, no less.

Deleted code is debugged code.

^ permalink  raw  reply  [nested|flat] 4+ messages in thread

* Re: remove pg_spin_delay() from atomics code
  2026-05-05 17:52 remove pg_spin_delay() from atomics code Nathan Bossart <nathandbossart@gmail.com>
  2026-05-27 06:37 ` Re: remove pg_spin_delay() from atomics code Corey Huinker <corey.huinker@gmail.com>
@ 2026-05-27 17:43   ` Nathan Bossart <nathandbossart@gmail.com>
  2026-06-30 15:58     ` Re: remove pg_spin_delay() from atomics code Nathan Bossart <nathandbossart@gmail.com>
  0 siblings, 1 reply; 4+ messages in thread

From: Nathan Bossart @ 2026-05-27 17:43 UTC (permalink / raw)
  To: Corey Huinker <corey.huinker@gmail.com>; +Cc: pgsql-hackers

On Wed, May 27, 2026 at 02:37:56AM -0400, Corey Huinker wrote:
> +1
> 
> The patch removes all 0 usages of pg_spin_delay, and all mentions of
> pg_spin_delay.
> 
> All the associated comments were removed, no more, no less.
> 
> Deleted code is debugged code.

Thanks for looking.  FWIW I'm planning to wait until v20 opens for
development before committing this.  That's probably unnecessary, but I see
no advantage to rushing.

-- 
nathan





^ permalink  raw  reply  [nested|flat] 4+ messages in thread

* Re: remove pg_spin_delay() from atomics code
  2026-05-05 17:52 remove pg_spin_delay() from atomics code Nathan Bossart <nathandbossart@gmail.com>
  2026-05-27 06:37 ` Re: remove pg_spin_delay() from atomics code Corey Huinker <corey.huinker@gmail.com>
  2026-05-27 17:43   ` Re: remove pg_spin_delay() from atomics code Nathan Bossart <nathandbossart@gmail.com>
@ 2026-06-30 15:58     ` Nathan Bossart <nathandbossart@gmail.com>
  0 siblings, 0 replies; 4+ messages in thread

From: Nathan Bossart @ 2026-06-30 15:58 UTC (permalink / raw)
  To: Corey Huinker <corey.huinker@gmail.com>; +Cc: pgsql-hackers

Committed.

-- 
nathan





^ permalink  raw  reply  [nested|flat] 4+ messages in thread


end of thread, other threads:[~2026-06-30 15:58 UTC | newest]

Thread overview: 4+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2026-05-05 17:52 remove pg_spin_delay() from atomics code Nathan Bossart <nathandbossart@gmail.com>
2026-05-27 06:37 ` Corey Huinker <corey.huinker@gmail.com>
2026-05-27 17:43   ` Nathan Bossart <nathandbossart@gmail.com>
2026-06-30 15:58     ` Nathan Bossart <nathandbossart@gmail.com>

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox