Re: Replace known_assigned_xids_lck by memory barrier - Mailing list pgsql-hackers

From Michail Nikolaev
Subject Re: Replace known_assigned_xids_lck by memory barrier
Date
Msg-id CANtu0ohnGXW6wfM7eXts=m23A1g9D7s1X0ms7bV3-x5oQugX5g@mail.gmail.com
Whole thread Raw
In response to Re: Replace known_assigned_xids_lck by memory barrier  (Nathan Bossart <nathandbossart@gmail.com>)
Responses Re: Replace known_assigned_xids_lck by memory barrier
List pgsql-hackers
Hello, Nathan.

> 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.

> 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.

> This use of pg_write_barrier() looks correct to me, but don't we need
> corresponding read barriers wherever we obtain the pointers? FWIW I tend
> to review src/backend/storage/lmgr/README.barrier in its entirety whenever
> I deal with this stuff.

Oh, yeah, you're right! (1)
I'll prepare an updated version of the patch soon. I don't why I was
assuming pg_write_barrier is enough (⊙_⊙')


[1]: https://github.com/postgres/postgres/blob/master/src/backend/storage/lmgr/README.barrier#L125



pgsql-hackers by date:

Previous
From: Alvaro Herrera
Date:
Subject: Re: cataloguing NOT NULL constraints
Next
From: Andy Fan
Date:
Subject: Avoid a potential unstable test case: xmlmap.sql