From: | Nathan Bossart <nathandbossart(at)gmail(dot)com> |
---|---|
To: | Michail Nikolaev <michail(dot)nikolaev(at)gmail(dot)com> |
Cc: | PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Subject: | Re: Replace known_assigned_xids_lck by memory barrier |
Date: | 2023-08-15 15:22:24 |
Message-ID: | 20230815152224.GA2296544@nathanxps13 |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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
From | Date | Subject | |
---|---|---|---|
Next Message | Nathan Bossart | 2023-08-15 15:34:23 | Re: pgbench - adding pl/pgsql versions of tests |
Previous Message | Russell Rose | Passfield Data Systems | 2023-08-15 15:06:16 | Converting sql anywhere to postgres |