Date: Wed, 8 Jan 2025 12:12:19 -0600
From: Nathan Bossart <nathandbossart@gmail.com>
To: pgsql-hackers@postgresql.org
Cc: dipiets@amazon.com
Subject: Re: use a non-locking initial test in TAS_SPIN on AArch64
Message-ID: <Z37AA6-SsJoLaifa@nathan>
References: <ZxgDEb_VpWyNZKB_@nathan>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <ZxgDEb_VpWyNZKB_@nathan>
Archived-At: <https://www.postgresql.org/message-id/Z37AA6-SsJoLaifa%40nathan>
Precedence: bulk

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