public inbox for [email protected]
help / color / mirror / Atom feed[PATCH v6] Highlight vacuum consideration in create index docs
7+ messages / 4 participants
[nested] [flat]
* [PATCH v6] Highlight vacuum consideration in create index docs
@ 2020-11-30 21:50 Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 7+ messages in thread
From: Alvaro Herrera @ 2020-11-30 21:50 UTC (permalink / raw)
Per James Coleman
---
doc/src/sgml/ref/create_index.sgml | 5 +++++
doc/src/sgml/ref/reindex.sgml | 5 +++++
2 files changed, 10 insertions(+)
diff --git a/doc/src/sgml/ref/create_index.sgml b/doc/src/sgml/ref/create_index.sgml
index 2054d5d943..d951f14b09 100644
--- a/doc/src/sgml/ref/create_index.sgml
+++ b/doc/src/sgml/ref/create_index.sgml
@@ -829,6 +829,11 @@ Indexes:
to remove an index.
</para>
+ <para>
+ Like any long-running transaction, <command>CREATE INDEX</command> can
+ affect which tuples can be removed by concurrent <command>VACUUM</command>.
+ </para>
+
<para>
Prior releases of <productname>PostgreSQL</productname> also had an
R-tree index method. This method has been removed because
diff --git a/doc/src/sgml/ref/reindex.sgml b/doc/src/sgml/ref/reindex.sgml
index 6e1cf06713..ef553f6481 100644
--- a/doc/src/sgml/ref/reindex.sgml
+++ b/doc/src/sgml/ref/reindex.sgml
@@ -436,6 +436,11 @@ Indexes:
CONCURRENTLY</command> cannot.
</para>
+ <para>
+ Like any long-running transaction, <command>REINDEX</command> can
+ affect which tuples can be removed by concurrent <command>VACUUM</command>.
+ </para>
+
<para>
<command>REINDEX SYSTEM</command> does not support
<command>CONCURRENTLY</command> since system catalogs cannot be reindexed
--
2.20.1
--+QahgC5+KEYLbs62--
^ permalink raw reply [nested|flat] 7+ messages in thread
* use a non-locking initial test in TAS_SPIN on AArch64
@ 2024-10-22 19:54 Nathan Bossart <[email protected]>
0 siblings, 2 replies; 7+ messages in thread
From: Nathan Bossart @ 2024-10-22 19:54 UTC (permalink / raw)
To: pgsql-hackers; +Cc: [email protected]
My colleague Salvatore Dipietro (CC'd) sent me a couple of profiles that
showed an enormous amount of s_lock() time going to the
__sync_lock_test_and_set() call in the AArch64 implementation of tas().
Upon closer inspection, I noticed that we don't implement a custom
TAS_SPIN() for this architecture, so I quickly hacked together the attached
patch and ran a couple of benchmarks that stressed the spinlock code. I
found no discussion about TAS_SPIN() on ARM in the archives, but I did
notice that the initial AArch64 support was added [0] before x86_64 started
using a non-locking test [1].
These benchmarks are for a c8g.24xlarge running a select-only pgbench with
256 clients and pg_stat_statements.track_planning enabled.
without the patch:
90.04% postgres [.] s_lock
1.07% pg_stat_statements.so [.] pgss_store
0.59% postgres [.] LWLockRelease
0.56% postgres [.] perform_spin_delay
0.31% [kernel] [k] arch_local_irq_enable
| while (TAS_SPIN(lock))
| {
| perform_spin_delay(&delayStatus);
0.12 |2c: -> bl perform_spin_delay
| tas():
| return __sync_lock_test_and_set(lock, 1);
0.01 |30: swpa w20, w1, [x19]
| s_lock():
99.87 | add x0, sp, #0x28
| while (TAS_SPIN(lock))
0.00 | ^ cbnz w1, 2c
tps = 74135.100891 (without initial connection time)
with the patch:
30.46% postgres [.] s_lock
5.88% postgres [.] perform_spin_delay
4.61% [kernel] [k] arch_local_irq_enable
3.31% [kernel] [k] next_uptodate_page
2.50% postgres [.] hash_search_with_hash_value
| while (TAS_SPIN(lock))
| {
| perform_spin_delay(&delayStatus);
0.63 |2c:+-->add x0, sp, #0x28
0.07 | |-> bl perform_spin_delay
| |while (TAS_SPIN(lock))
0.25 |34:| ldr w0, [x19]
65.19 | +--cbnz w0, 2c
| tas():
| return __sync_lock_test_and_set(lock, 1);
0.00 | swpa w20, w0, [x19]
| s_lock():
33.82 | ^ cbnz w0, 2c
tps = 549462.785554 (without initial connection time)
[0] https://postgr.es/c/5c7603c
[1] https://postgr.es/c/b03d196
--
nathan
From a69b7d38c40f0fb49f714c49bb45c292e7c8587d Mon Sep 17 00:00:00 2001
From: Nathan Bossart <[email protected]>
Date: Tue, 22 Oct 2024 14:33:39 -0500
Subject: [PATCH v1 1/1] Use a non-locking initial test in TAS_SPIN on AArch64.
---
src/include/storage/s_lock.h | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/src/include/storage/s_lock.h b/src/include/storage/s_lock.h
index e94ed5f48b..07f343baf8 100644
--- a/src/include/storage/s_lock.h
+++ b/src/include/storage/s_lock.h
@@ -270,6 +270,12 @@ tas(volatile slock_t *lock)
*/
#if defined(__aarch64__)
+/*
+ * On ARM64, it's a win to use a non-locking test before the TAS proper. It
+ * may be a win on 32-bit ARM, too, but nobody's tested it yet.
+ */
+#define TAS_SPIN(lock) (*(lock) ? 1 : TAS(lock))
+
#define SPIN_DELAY() spin_delay()
static __inline__ void
--
2.39.5 (Apple Git-154)
Attachments:
[text/plain] v1-0001-Use-a-non-locking-initial-test-in-TAS_SPIN-on-AAr.patch (870B, ../../ZxgDEb_VpWyNZKB_@nathan/2-v1-0001-Use-a-non-locking-initial-test-in-TAS_SPIN-on-AAr.patch)
download | inline diff:
From a69b7d38c40f0fb49f714c49bb45c292e7c8587d Mon Sep 17 00:00:00 2001
From: Nathan Bossart <[email protected]>
Date: Tue, 22 Oct 2024 14:33:39 -0500
Subject: [PATCH v1 1/1] Use a non-locking initial test in TAS_SPIN on AArch64.
---
src/include/storage/s_lock.h | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/src/include/storage/s_lock.h b/src/include/storage/s_lock.h
index e94ed5f48b..07f343baf8 100644
--- a/src/include/storage/s_lock.h
+++ b/src/include/storage/s_lock.h
@@ -270,6 +270,12 @@ tas(volatile slock_t *lock)
*/
#if defined(__aarch64__)
+/*
+ * On ARM64, it's a win to use a non-locking test before the TAS proper. It
+ * may be a win on 32-bit ARM, too, but nobody's tested it yet.
+ */
+#define TAS_SPIN(lock) (*(lock) ? 1 : TAS(lock))
+
#define SPIN_DELAY() spin_delay()
static __inline__ void
--
2.39.5 (Apple Git-154)
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: use a non-locking initial test in TAS_SPIN on AArch64
@ 2024-10-23 03:01 Jingtang Zhang <[email protected]>
parent: Nathan Bossart <[email protected]>
1 sibling, 1 reply; 7+ messages in thread
From: Jingtang Zhang @ 2024-10-23 03:01 UTC (permalink / raw)
To: Nathan Bossart <[email protected]>; +Cc: [email protected]
Hi~
Upon closer inspection, I noticed that we don't implement a custom
> TAS_SPIN() for this architecture, so I quickly hacked together the attached
> patch and ran a couple of benchmarks that stressed the spinlock code. I
> found no discussion about TAS_SPIN() on ARM in the archives, but I did
> notice that the initial AArch64 support was added [0] before x86_64 started
> using a non-locking test [1].
>
It reminds me of a discussion about improving spinlock performance on ARM
in 2020 [0], though the discussion is about CAS and TAS, not TAS_SPIN()
itself.
> tps = 74135.100891 (without initial connection time)
> tps = 549462.785554 (without initial connection time)
The result looks great, but the discussion in [0] shows that the result may
vary among different ARM chips. Could you provide the chip model of this
test? So that we can do a cross validation of this patch. Not sure if
compiler
version is necessary too. I'm willing to test it on Alibaba Cloud Yitian 710
if I have time.
[0]
https://www.postgresql.org/message-id/flat/CAPpHfdsGqVd6EJ4mr_RZVE5xSiCNBy4MuSvdTrKmTpM0eyWGpg%40mai...
- Regards
Jingtang
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: use a non-locking initial test in TAS_SPIN on AArch64
@ 2025-01-08 18:12 Nathan Bossart <[email protected]>
parent: Nathan Bossart <[email protected]>
1 sibling, 1 reply; 7+ messages in thread
From: Nathan Bossart @ 2025-01-08 18:12 UTC (permalink / raw)
To: pgsql-hackers; +Cc: [email protected]
On Tue, Oct 22, 2024 at 02:54:57PM -0500, Nathan Bossart wrote:
> My colleague Salvatore Dipietro (CC'd) sent me a couple of profiles that
> showed an enormous amount of s_lock() time going to the
> __sync_lock_test_and_set() call in the AArch64 implementation of tas().
> Upon closer inspection, I noticed that we don't implement a custom
> TAS_SPIN() for this architecture, so I quickly hacked together the attached
> patch and ran a couple of benchmarks that stressed the spinlock code. I
> found no discussion about TAS_SPIN() on ARM in the archives, but I did
> notice that the initial AArch64 support was added [0] before x86_64 started
> using a non-locking test [1].
>
> These benchmarks are for a c8g.24xlarge running a select-only pgbench with
> 256 clients and pg_stat_statements.track_planning enabled.
>
> without the patch:
>
> [...]
>
> tps = 74135.100891 (without initial connection time)
>
> with the patch:
>
> [...]
>
> tps = 549462.785554 (without initial connection time)
Are there any objections to proceeding with this change? So far, it's been
tested on a c8g.24xlarge and an Apple M3 (which seems to be too small to
show any effect). If anyone has access to a larger ARM machine, additional
testing would be greatly appreciated. I think it would be unfortunate if
this slipped to v19.
--
nathan
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: use a non-locking initial test in TAS_SPIN on AArch64
@ 2025-01-08 20:23 Tom Lane <[email protected]>
parent: Nathan Bossart <[email protected]>
0 siblings, 0 replies; 7+ messages in thread
From: Tom Lane @ 2025-01-08 20:23 UTC (permalink / raw)
To: Nathan Bossart <[email protected]>; +Cc: pgsql-hackers; [email protected]
Nathan Bossart <[email protected]> writes:
> Are there any objections to proceeding with this change? So far, it's been
> tested on a c8g.24xlarge and an Apple M3 (which seems to be too small to
> show any effect). If anyone has access to a larger ARM machine, additional
> testing would be greatly appreciated. I think it would be unfortunate if
> this slipped to v19.
I just acquired an M4 Pro, which may also be too small to show any
effect, but perhaps running the test there would at least give us
more confidence that there's not a bad effect. Which test case(s)
would you recommend trying?
regards, tom lane
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: use a non-locking initial test in TAS_SPIN on AArch64
@ 2025-01-15 11:50 Jingtang Zhang <[email protected]>
parent: Jingtang Zhang <[email protected]>
0 siblings, 1 reply; 7+ messages in thread
From: Jingtang Zhang @ 2025-01-15 11:50 UTC (permalink / raw)
To: Nathan Bossart <[email protected]>; +Cc: [email protected]
Hi, Nathan.
I just realized that I almost forgot about this thread :)
> The result looks great, but the discussion in [0] shows that the result may
> vary among different ARM chips. Could you provide the chip model of this
> test? So that we can do a cross validation of this patch. Not sure if compiler
> version is necessary too. I'm willing to test it on Alibaba Cloud Yitian 710
> if I have time.
I did some benchmark on Yitian 710.
On c8y.16xlarge (64 cores):
Without the patch:
80.31% postgres [.] __aarch64_swp4_acq
1.77% postgres [.] __aarch64_ldadd4_acq_rel
1.13% postgres [.] hash_search_with_hash_value
0.87% pg_stat_statements.so [.] __aarch64_swp4_acq
0.72% postgres [.] perform_spin_delay
0.44% postgres [.] _bt_compare
tps = 295272.628421 (including connections establishing)
tps = 295335.660323 (excluding connections establishing)
Patched:
9.94% postgres [.] s_lock
6.07% postgres [.] __aarch64_swp4_acq
5.73% postgres [.] hash_search_with_hash_value
2.81% postgres [.] perform_spin_delay
2.29% postgres [.] _bt_compare
2.15% postgres [.] PinBuffer
tps = 864519.764125 (including connections establishing)
tps = 864638.244443 (excluding connections establishing)
Seems that great performance could be gained if s_lock contention is severe.
This may be more likely to happen on bigger machines.
On c8y.2xlarge (8 cores), I failed to make s_lock contended severely, and
as a result this patch didn’t bring any difference outside the noise.
Regards,
Jingtang
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: use a non-locking initial test in TAS_SPIN on AArch64
@ 2025-01-15 18:50 Nathan Bossart <[email protected]>
parent: Jingtang Zhang <[email protected]>
0 siblings, 0 replies; 7+ messages in thread
From: Nathan Bossart @ 2025-01-15 18:50 UTC (permalink / raw)
To: Jingtang Zhang <[email protected]>; +Cc: [email protected]
On Wed, Jan 15, 2025 at 07:50:38PM +0800, Jingtang Zhang wrote:
> Seems that great performance could be gained if s_lock contention is severe.
> This may be more likely to happen on bigger machines.
>
> On c8y.2xlarge (8 cores), I failed to make s_lock contended severely, and
> as a result this patch didn´t bring any difference outside the noise.
Thanks for sharing.
--
nathan
^ permalink raw reply [nested|flat] 7+ messages in thread
end of thread, other threads:[~2025-01-15 18:50 UTC | newest]
Thread overview: 7+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-11-30 21:50 [PATCH v6] Highlight vacuum consideration in create index docs Alvaro Herrera <[email protected]>
2024-10-22 19:54 use a non-locking initial test in TAS_SPIN on AArch64 Nathan Bossart <[email protected]>
2024-10-23 03:01 ` Re: use a non-locking initial test in TAS_SPIN on AArch64 Jingtang Zhang <[email protected]>
2025-01-15 11:50 ` Re: use a non-locking initial test in TAS_SPIN on AArch64 Jingtang Zhang <[email protected]>
2025-01-15 18:50 ` Re: use a non-locking initial test in TAS_SPIN on AArch64 Nathan Bossart <[email protected]>
2025-01-08 18:12 ` Re: use a non-locking initial test in TAS_SPIN on AArch64 Nathan Bossart <[email protected]>
2025-01-08 20:23 ` Re: use a non-locking initial test in TAS_SPIN on AArch64 Tom Lane <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox