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

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Andres Freund <andres(at)anarazel(dot)de>, 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 14:11:06
Message-ID: CA+TgmoY9VeZdTZkTp8SKoXPKKj0PGu2aKPt0dLTDDsgu+vm0yw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers pgsql-hackers

On Wed, Apr 13, 2016 at 9:52 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>> I have never understood why you didn't include 64-bit atomics in the
>> original atomics implementation, and I really think we should have
>> committed a patch to add them long before now.
>
> What will you do on 32-bit platforms (or, more generally, anything
> lacking 64-bit-wide atomics)?

We fall back to emulating it using spinlocks. This isn't really an
issue in practice because 32-bit x86 has native 64-bit atomics, and
it's hard to point to another 32-bit platform that is likely to be
have enough concurrency for the lack of 64-bit atomics to matter.
Actually, it looks like we have 64-bit atomics in the tree already;
it's only the fallback implementation that is missing (so anything you
do using 64-bit atomics would need an alternate implementation that
did not rely on them).

But the really interesting that the patch to which Andres linked does
is introduce machinery to try to determine whether a platform has
8-byte single-copy atomicity; that is, whether a load or store of an
aligned 8-byte value is guaranteed not to be torn. We currently avoid
assuming that, but this requires additional spinlocks in a significant
number of places; the regression seen using "snapshot too old" at high
concurrency is merely the tip of the iceberg. And the annoying thing
about avoiding that assumption is that it actually is true on pretty
much every modern platform. Look at this gem Andres wrote in that
patch:

+/*
+ * 8 byte reads / writes have single-copy atomicity on 32 bit x86 platforms
+ * since at least the 586. As well as on all x86-64 cpus.
+ */
+#if defined(__i568__) || defined(__i668__) || /* gcc i586+ */ \
+ (defined(_M_IX86) && _M_IX86 >= 500) || /* msvc i586+ */ \
+ defined(__x86_64__) || defined(__x86_64) || defined(_M_X64) /*
gcc, sunpro, msvc */
+#define PG_HAVE_8BYTE_SINGLE_COPY_ATOMICITY
+#endif /* 8 byte single-copy atomicity */

I don't know if that test is actually correct, and I wonder about
compile-time environment vs. run-time environment, but I have my
doubts about how well PostgreSQL 9.6 would run on an i486. I doubt
that is the platform for which we should be optimizing.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

In response to

Responses

Browse pgsql-committers by date

  From Date Subject
Next Message Tom Lane 2016-04-13 14:20:37 Re: Re: [COMMITTERS] pgsql: Avoid extra locks in GetSnapshotData if old_snapshot_threshold <
Previous Message Alvaro Herrera 2016-04-13 14:08:21 Re: Re: [COMMITTERS] pgsql: Avoid extra locks in GetSnapshotData if old_snapshot_threshold <

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2016-04-13 14:11:30 Re: Missing PG_INT32_MIN in numutils.c
Previous Message Tom Lane 2016-04-13 14:09:49 Re: Html parsing and inline elements