public inbox for [email protected]
help / color / mirror / Atom feedFrom: Nathan Bossart <[email protected]>
To: Michail Nikolaev <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Cc: Tom Lane <[email protected]>
Subject: Re: Replace known_assigned_xids_lck by memory barrier
Date: Tue, 15 Aug 2023 08:22:24 -0700
Message-ID: <20230815152224.GA2296544@nathanxps13> (raw)
In-Reply-To: <CANtu0ohnGXW6wfM7eXts=m23A1g9D7s1X0ms7bV3-x5oQugX5g@mail.gmail.com>
References: <CANtu0oh0si=jG5z_fLeFtmYcETssQ08kLEa8b6TQqDm_cinroA@mail.gmail.com>
<20230814153634.GB1395427@nathanxps13>
<CANtu0ohnGXW6wfM7eXts=m23A1g9D7s1X0ms7bV3-x5oQugX5g@mail.gmail.com>
On Tue, Aug 15, 2023 at 12:29:24PM +0200, Michail Nikolaev wrote:
>> What sort of benefits do you see from this patch? It might be worthwhile
>> in itself to remove spinlocks when possible, but IME it's much easier to
>> justify such changes when there is a tangible benefit we can point to.
>
> Oh, it is not an easy question :)
>
> The answer, probably, looks like this:
> 1) performance benefits of spin lock acquire removing in
> KnownAssignedXidsGetOldestXmin and KnownAssignedXidsSearch
> 2) it is closing 13-year-old tech depth
>
> But in reality, it is not easy to measure performance improvement
> consistently for this change.
Okay. Elsewhere, it seems like folks are fine with patches that reduce
shared memory space via atomics or barriers even if there's no immediate
benefit [0], so I think it's fine to proceed.
>> Are the assignments in question guaranteed to be atomic? IIUC we assume
>> that aligned 4-byte loads/stores are atomic, so we should be okay as long
>> as we aren't handling anything larger.
>
> Yes, 4-bytes assignment are atomic, locking is used to ensure memory
> write ordering in this place.
Yeah, it looks like both the values that are protected by
known_assigned_xids_lck are integers, so this should be okay. One
remaining question I have is whether it is okay if we see an updated value
for one of the head/tail variables but not the other. It looks like the
tail variable is only updated with ProcArrayLock held exclusively, which
IIUC wouldn't prevent such mismatches even today, since we use a separate
spinlock for reading them in some cases.
[0] https://postgr.es/m/20230524214958.mt6f5xokpumvnrio%40awork3.anarazel.de
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
view thread (5+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected]
Subject: Re: Replace known_assigned_xids_lck by memory barrier
In-Reply-To: <20230815152224.GA2296544@nathanxps13>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox