Re: Re: [COMMITTERS] pgsql: Avoid extra locks in GetSnapshotData if old_snapshot_threshold <

From: Andres Freund <andres(at)anarazel(dot)de>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Noah Misch <noah(at)leadboat(dot)com>, Kevin Grittner <kgrittn(at)gmail(dot)com>, Kevin Grittner <kgrittn(at)postgresql(dot)org>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Re: [COMMITTERS] pgsql: Avoid extra locks in GetSnapshotData if old_snapshot_threshold <
Date: 2016-04-13 15:18:01
Message-ID: 20160413151801.nafgcxlitagi6fec@alap3.anarazel.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers pgsql-hackers

On 2016-04-13 10:42:03 -0400, Tom Lane wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> > On Wed, Apr 13, 2016 at 10:20 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> >> That's what I thought you were going to say, and it means that any
> >> "performance improvement" patch that relies on 64-bit atomics in hotspot
> >> code paths is going to be a complete disaster on anything but modern Intel
> >> hardware. I'm not sure that's a direction we want to go in. We need to
> >> stick to a set of atomics that's pretty widely portable.
>
> > I think 64-bit atomics *are* pretty widely portable. Can you name a
> > system with more than 4 CPU cores that doesn't support them?
>
> No, you're ignoring my point, which is what happens on single-CPU
> 32-bit machines, and whether we aren't going to destroy performance
> on low-end machines in pursuit of better performance on high-end.

I think generally the only platform of concern wrt is arm (< armv8),
which doesn't have 64bit atomicity and doesn't have
single-copy-atomicity for 8 byte values either (C.f.
https://wiki.postgresql.org/wiki/Atomics). But:

> Now, to the extent that a patch uses a 64-bit atomic op to replace
> a spinlock acquisition, it might be pretty much a wash if low-end
> machines have to use a spinlock to emulate the atomic op. But it
> would be really easy for the translation to replace one spinlock
> acquisition with multiple spinlock acquisitions, and that would hurt.

Which is why I'm actually proposing to *not* use a pg_atomic_uint64, just a
single define to remove the spinlock acquisition:
http://archives.postgresql.org/message-id/20160413150839.mevdlgekizxyjhc5%40alap3.anarazel.de

I think there are a number of LSNs which we'd be better of replacing LSN
manipulations with an actual atomic operation (including fallback to the
spinlock) might be beneficial. But that'd be a larger patch, and would
require more testing; which is why I'm proposing the above.

Greetings,

Andres Freund

In response to

Responses

Browse pgsql-committers by date

  From Date Subject
Next Message Robert Haas 2016-04-13 15:27:09 Re: Re: [COMMITTERS] pgsql: Avoid extra locks in GetSnapshotData if old_snapshot_threshold <
Previous Message Andres Freund 2016-04-13 15:08:39 Re: Re: [COMMITTERS] pgsql: Avoid extra locks in GetSnapshotData if old_snapshot_threshold <

Browse pgsql-hackers by date

  From Date Subject
Next Message José Luis Tallón 2016-04-13 15:21:52 Re: Parser extensions (maybe for 10?)
Previous Message Andres Freund 2016-04-13 15:08:39 Re: Re: [COMMITTERS] pgsql: Avoid extra locks in GetSnapshotData if old_snapshot_threshold <