Subject: Re: [PATCH] audo-detect and use -moutline-atomics compilation flag for aarch64

From: "Pop, Sebastian" <[email-masked]>
Date: Tue, 26 Oct 2021 04:04:03 +0000

?Hi Christoph,

This is a follow-up email based on recommendations from Heikki Linnakangas:

https://www.postgresql.org/message-id/5f048a5a-c471-50ae-30ad-385dcafc616b%40iki.fi

and Tom Lane:

https://www.postgresql.org/message-id/888692.1601489331%40sss.pgh.pa.us

> Maybe you should speak to the distribution vendors or the folk packaging PostgreSQL for those distributions, instead.

Following the initial post by Tsahi Zidenberg:

https://www.postgresql.org/message-id/099F69EE-51D3-4214-934A-1F28C0A1A7A7%40amazon.com

> The patch was tested to improve pgbench simple-update by 10% and sysbench write-only by 3% on a 64-core armv8.2 machine (AWS m6g.16xlarge).

?

?The change is to compile PostgreSQL for arm64 with an extra flag -moutline-atomics when the compiler is gcc-8.5 or gcc-9.4.

-moutline-atomics is enabled by default in all newer compilers gcc-{10, 11, trunk}, and on clang-{12, 13, trunk}.

The change is needed only for a limited number of Debian and Ubuntu releases compiled by gcc versions with the outline-atomics flag disabled by default.

Would it be possible to update the PostgreSQL arm64 apt packages for Debian and Ubuntu with the -moutline-atomics flag?

Thanks,
Sebastian

---

From: Christoph Berg <[email-masked]>
Date: Tue, 26 Oct 2021 17:09:26 +0200

Re: Pop, Sebastian
> https://www.postgresql.org/message-id/888692.1601489331%40sss.pgh.pa.us

Well I wouldn't exactly call these messages "recommendations".

> The change is needed only for a limited number of Debian and Ubuntu releases compiled by gcc versions with the outline-atomics flag disabled by default.

None of the Debian releases are using 8.5 or 9.4, and on the Ubuntu
side focal is using 9.3. Is that gcc version even affected?

Christoph

---

From: "Pop, Sebastian" <[email-masked]>
Date: Tue, 26 Oct 2021 15:16:08 +0000

> on the Ubuntu side focal is using 9.3. Is that gcc version even affected?

Yes, we have backported the outline-atomics flag to Ubuntu 20.04 gcc-9.3.

On arm64 you can check whether a compiler has the option with:
$ gcc -v --help -Q 2>/dev/null | grep outline-atomics
  -moutline-atomics           		[disabled]
$ gcc --version
gcc (Ubuntu 9.3.0-17ubuntu1~20.04) 9.3.0
[...]

Sebastian

---

From: Christoph Berg <[email-masked]>
Date: Tue, 26 Oct 2021 17:41:25 +0200

Re: Pop, Sebastian
> gcc (Ubuntu 9.3.0-17ubuntu1~20.04) 9.3.0

Not here:

$ gcc --version

Did you rebuild the package locally without changing the Debian
version?

Christoph

---

From: "Pop, Sebastian" <[email-masked]>
Date: Tue, 26 Oct 2021 16:39:58 +0000

> $ gcc -v --help -Q 2>/dev/null | grep outline-atomics

This needs to be on an arm64 machine.
The outline-atomics flag does not exist when targeting x86_64.
The flag has been back-ported to Ubuntu 20.04 a year and a half ago: IIRC, it went in just before the release in April 2020.

> Did you rebuild the package locally without changing the Debian version?

I have not re-compiled gcc.  The gcc-9.3 is the one distributed as part of Ubuntu 20.04.

If you need access to Graviton2 instances, we have a system of credits that we can use to fund compute time for open-source CI projects.
Let me know if you are interested in getting those compute credits, and I will take the request to my management.

Sebastian

---

From: "Pop, Sebastian" <[email-masked]>
Date: Tue, 9 Nov 2021 22:32:00 +0000

Hi Christoph,

Could you please let me know what is the process to enable -moutline-atomics for arm64 binaries distributed by PostgreSQL for Ubuntu 20.04?
Using the right LSE atomic instructions is very important for the performance of PostgreSQL on all new arm64 processors.
Please let me know how I can help to enable the flag, test, and benchmark the new packages.

Thanks,
Sebastian

---

From: Christoph Berg <[email-masked]>
Date: Thu, 11 Nov 2021 17:55:58 +0100

Re: Pop, Sebastian
> This needs to be on an arm64 machine.

Oh, sure. Sorry I was missing a lot of context that wasn't immediately
clear.

> The flag has been back-ported to Ubuntu 20.04 a year and a half ago: IIRC, it went in just before the release in April 2020.

It is indeed present there, but not enabled:

$ for chroot in *; do echo $chroot:; schroot -c $chroot -- gcc -v --help -Q 2>&1 | grep outline-atomics; done
bionic-arm64:
bookworm-arm64:
  -moutline-atomics           		[enabled]
bullseye-arm64:
  -moutline-atomics           		[enabled]
buster-arm64:
focal-arm64:
  -moutline-atomics           		[disabled]
sid-arm64:
  -moutline-atomics           		[enabled]

The (newer) Debian dists have it enabled by default.

> I have not re-compiled gcc.  The gcc-9.3 is the one distributed as part of Ubuntu 20.04.

Again, that was a lot of context I didn't have.

> Let me know if you are interested in getting those compute credits, and I will take the request to my management.

Thanks for the offer - we already have a arm64 build node sponsored by
Huawei that does the job for now.

Re: Pop, Sebastian
> Please let me know how I can help to enable the flag, test, and benchmark the new packages.

First, we need to be convinced it's a good idea. From that I got from
the messages you were linking to, Heikki "definitely do[es]n't want to
override that decision", and Tom doesn't "find this argument terribly
convincing".

The benchmarks in
https://www.postgresql.org/message-id/1C8D0E58-FB33-4105-AC00-8FA07621F5DD%40amazon.com
don't look like the option is a game-changer.

Even if we are convinced, we don't have the infrastructure in the
package yet to do architecture- and distribution-specific
optimizations. Would you (Amazon?) be willing to sponsor the required
package engineering?

Christoph

---

From: "Pop, Sebastian" <[email-masked]>
Date: Thu, 11 Nov 2021 17:11:02 +0000

> don't look like the option is a game-changer.

Since then we have seen benchmarks that are several orders of magnitude faster with LSE instructions.
I will get the new data and share.

> optimizations. Would you (Amazon?) be willing to sponsor the required

Yes, I am willing to do whatever is necessary to enable the flag on PostgreSQL builds for Ubuntu 20.04.
Please point me to the (git?) repo and the files that need to be changed.

Thanks,
Sebastian

---

From: Christoph Berg <[email-masked]>
Date: Thu, 11 Nov 2021 18:13:35 +0100

Re: Pop, Sebastian
> Please point me to the (git?) repo and the files that need to be changed.

https://salsa.debian.org/postgresql/postgresql-common.git
https://salsa.debian.org/postgresql/postgresql.git
https://git.postgresql.org/gitweb/?p=pgapt.git;a=summary

Christoph

---

From: "Saidi, Ali" <[email-masked]>
Date: Thu, 11 Nov 2021 17:22:19 +0000

On 11/11/21, 11:14 AM, "Christoph Berg" <[email-masked]> wrote:

    CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe.

    Re: Pop, Sebastian
    > Please point me to the (git?) repo and the files that need to be changed.

The binary compiled with -moutline-atomics works for all arm64 architecture versions. I don't believe there would be new package engineering required unless I'm missing something, but if there is, yes, we're absolutely happy to help. 

Thanks,
Ali

---

From: "Pop, Sebastian" <[email-masked]>
Date: Thu, 11 Nov 2021 17:38:03 +0000

Thanks Christoph for the pointers.

I see that the source code of PostgreSQL is patched before builds:
i.e., the configure.in file has this patch applied to it https://salsa.debian.org/postgresql/postgresql/-/blob/13/debian/patches/autoconf2.69

Following the original patch from Tsahi Zidenberg,
https://www.postgresql.org/message-id/attachment/111983/0001-Support-outline-atomics-on-aarch64.patch
I would like to add the attached patch to the list of patches applied before build.

Christoph, could you please let me know how I should test the patch?

Thanks,
Sebastian

From 8e959e3b96c880004626cfe5d1b8666c52ae9ba7 Mon Sep 17 00:00:00 2001
From: Sebastian Pop <[email-masked]>
Date: Thu, 11 Nov 2021 17:34:12 +0000
Subject: [PATCH] [arm64] enable -moutline-atomics flag

---
 debian/patches/arm64-outline-atomics | 13 +++++++++++++
 1 file changed, 13 insertions(+)
 create mode 100644 debian/patches/arm64-outline-atomics

diff --git a/debian/patches/arm64-outline-atomics b/debian/patches/arm64-outline-atomics
new file mode 100644
index 0000000..741b243
--- /dev/null
+++ b/debian/patches/arm64-outline-atomics
@@ -0,0 +1,13 @@
+--- a/configure.in
++++ b/configure.in
+@@ -532,6 +532,10 @@ if test "$GCC" = yes -a "$ICC" = no; then
+   if test -n "$NOT_THE_CFLAGS"; then
+     CFLAGS="$CFLAGS -Wno-stringop-truncation"
+   fi
++  if test x"$host_cpu" == x"aarch64"; then
++    PGAC_PROG_CC_CFLAGS_OPT([-moutline-atomics])
++    PGAC_PROG_CXX_CFLAGS_OPT([-moutline-atomics])
++  fi
+ elif test "$ICC" = yes; then
+   # Intel's compiler has a bug/misoptimization in checking for
+   # division by NAN (NaN == 0), -mp1 fixes it, so add it to the CFLAGS.
-- 
2.25.1

---

From: Christoph Berg <[email-masked]>
Date: Thu, 11 Nov 2021 22:02:30 +0100

Re: Pop, Sebastian
> I would like to add the attached patch to the list of patches applied before build.

That will fail on bionic.

> Christoph, could you please let me know how I should test the patch?

Apply it, and run "make check-world" on focal. The others should be
unaffected since the flag is on by default.

Christoph

---

From: "Pop, Sebastian" <[email-masked]>
Date: Thu, 11 Nov 2021 21:21:30 +0000

> I will get the new data and share.

On an m6gd.metal with PostgreSQL v11
test: pgbench - select-only
Measure tps (transactions/s), higher is better.

build without LSE: 132143 tps 
with LSE atomics: 844552 tps

That is a 6.39x speedup.

Sebastian

---

From: "Pop, Sebastian" <[email-masked]>
Date: Thu, 11 Nov 2021 21:25:28 +0000

> That will fail on bionic.

The autoconf code does a check that the compiler supports the -moutline-atomics flag.
The change will still be fine on bionic and all other distros with a gcc without the flag.

> unaffected since the flag is on by default.

Thanks for the instructions.  I will test the patch and let you know.

Sebastian

---

From: "Pop, Sebastian" <[email-masked]>
Date: Thu, 11 Nov 2021 21:54:05 +0000

> unaffected since the flag is on by default.

Here are the steps in my testing on focal - ubuntu 20.04:

git clone https://github.com/postgres/postgres
cd postgres
patch -p1 < a.diff
autoconf
make
make check-world

All checks passed on a c6g Graviton2 instance.

configure.in was renamed configure.ac in July 2020.
For older postgresql versions we need to patch configure.in.
See attached the patch a.diff that I used on trunk postgresql.

Thanks,
Sebastian

--- a/configure.ac
+++ b/configure.ac
@@ -549,6 +549,10 @@ if test "$GCC" = yes -a "$ICC" = no; then
   if test -n "$NOT_THE_CFLAGS"; then
     CFLAGS="$CFLAGS -Wno-stringop-truncation"
   fi
+  if test x"$host_cpu" == x"aarch64"; then
+    PGAC_PROG_CC_CFLAGS_OPT([-moutline-atomics])
+    PGAC_PROG_CXX_CFLAGS_OPT([-moutline-atomics])
+  fi
 elif test "$ICC" = yes; then
   # Intel's compiler has a bug/misoptimization in checking for
   # division by NAN (NaN == 0), -mp1 fixes it, so add it to the CFLAGS.

---

From: "Pop, Sebastian" <[email-masked]>
Date: Thu, 11 Nov 2021 22:26:34 +0000

sudo apt install build-essential libreadline-dev zlib1g-dev bison flex
> git clone https://github.com/postgres/postgres
./configure
> All checks passed on a c6g Graviton2 instance.

I also run the above steps on bionic - ubuntu 18.04 - to check that the patch passes when gcc does not have the -moutline-atomics flag.
All checks pass on bionic on a c6g instance.
Thanks Ali for recommending the extra test.

Sebastian

---

From: "Pop, Sebastian" <[email-masked]>
Date: Tue, 16 Nov 2021 00:02:42 +0000

Hi Christoph,

a quick follow-up email to make sure you have everything you need to enable -moutline-atomics flag on Ubuntu 20.04 builds of PostgreSQL.
Please let me know if you need more experiments or tests with the patch.

Thanks,
Sebastian

---

From: Christoph Berg <[email-masked]>
Date: Tue, 16 Nov 2021 11:30:10 +0100

Re: Pop, Sebastian
> Thanks Ali for recommending the extra test.

Thanks for verifying that part as well, I'll try throwing in the patch
now.

Christoph

---

From: Christoph Berg <[email-masked]>
Date: Tue, 16 Nov 2021 14:50:34 +0100

Re: To Pop, Sebastian
> Thanks for verifying that part as well, I'll try throwing in the patch

Things are looking good on PG14, but PG10 and 9.6 fail:

configure.in:485: error: possibly undefined macro: PGAC_PROG_CXX_CFLAGS_OPT

https://pgdgbuild.dus.dg-i.net/job/postgresql-10-binaries/architecture=ppc64el,distribution=focal/76/console

So we'll have it on PG 11 .. 14, unless the regression tests act up.

Christoph

---

From: "Pop, Sebastian" <[email-masked]>
Date: Tue, 16 Nov 2021 13:57:15 +0000

> configure.in:485: error: possibly undefined macro: PGAC_PROG_CXX_CFLAGS_OPT

Thanks Christoph for the heads up.
I will investigate the autoconf issue and send you a patch specific to PG10 and 9.
Most likely the PGAC_PROG_CXX_CFLAGS_OPT macro has been introduced after these releases.

Sebastian

---

From: "Pop, Sebastian" <[email-masked]>
Date: Tue, 16 Nov 2021 14:31:50 +0000

> Most likely the PGAC_PROG_CXX_CFLAGS_OPT macro has been introduced after these releases.

The above macro has been added in March 2018:
https://github.com/postgres/postgres/commit/6869b4f2584787d9e4cefaab8a4bae1ecbe63766
the patch also adds CXXFLAGS, and so before this patch there is no need to detect CXXFLAGS.

Please see attached a patch tested on Ubuntu 20.04 on the following branches:
remotes/origin/REL9_6_STABLE
remotes/origin/REL_10_STABLE

I checked that REL_11_STABLE has the CXXFLAGS and the above macro.

Thanks,
Sebastian
diff --git a/configure.in b/configure.in
index 0ef9a3cca6..6dcedb68a3 100644
--- a/configure.in
+++ b/configure.in
@@ -475,6 +475,9 @@ if test "$GCC" = yes -a "$ICC" = no; then
   if test -n "$NOT_THE_CFLAGS"; then
     CFLAGS="$CFLAGS -Wno-stringop-truncation"
   fi
+  if test x"$host_cpu" == x"aarch64"; then
+    PGAC_PROG_CC_CFLAGS_OPT([-moutline-atomics])
+  fi
 elif test "$ICC" = yes; then
   # Intel's compiler has a bug/misoptimization in checking for
   # division by NAN (NaN == 0), -mp1 fixes it, so add it to the CFLAGS.

---

From: "Pop, Sebastian" <[email-masked]>
Date: Tue, 16 Nov 2021 21:04:25 +0000

Thanks Christoph for the quick turn-around.

I see that builds with -moutline-atomics succeeded for Focal arm64 builds for all the branches:
https://pgdgbuild.dus.dg-i.net/view/PostgreSQL/job/postgresql-9.6-binaries/architecture=arm64,distribution=focal/88/
https://pgdgbuild.dus.dg-i.net/job/postgresql-10-binaries/77/architecture=arm64,distribution=focal/
https://pgdgbuild.dus.dg-i.net/view/PostgreSQL/job/postgresql-11-binaries/architecture=arm64,distribution=focal/686/
https://pgdgbuild.dus.dg-i.net/view/PostgreSQL/job/postgresql-12-binaries/architecture=arm64,distribution=focal/875/
https://pgdgbuild.dus.dg-i.net/view/PostgreSQL/job/postgresql-13-binaries/architecture=arm64,distribution=focal/858/
https://pgdgbuild.dus.dg-i.net/view/PostgreSQL/job/postgresql-14-binaries/architecture=arm64,distribution=focal/651/

I see that the patch is still missing on branch 15 that is the development branch:
https://pgdgbuild.dus.dg-i.net/view/PostgreSQL/job/postgresql-15-binaries/architecture=amd64,distribution=focal/345/
Maybe we need to add the patch to that branch as well?

How are the above binaries distributed to users?
I see for instance that pgsql 14 binaries have last been updated in 30-Sep-2021:
https://apt.postgresql.org/pub/repos/apt/dists/focal-pgdg/14/binary-arm64/
Is there another step needed to make those binaries available through the PostgreSQL Apt Repository ( https://www.postgresql.org/download/linux/ubuntu/ )

Does Canonical rely on these builds to update the Focal PostgreSQL packages?

Thanks,
Sebastian

---

From: Aaron Pavely <[email-masked]>
Date: Wed, 17 Nov 2021 18:00:38 -0600

On Tue, 16 Nov, 2021 at 15:04 Pop, Sebastian <[email-masked]> wrote:

> Is there another step needed to make those binaries available through the
> PostgreSQL Apt Repository (
> https://www.postgresql.org/download/linux/ubuntu/ )
>

Sebastian,

I see new arm64 packages in the debian repository pool, e.g.:
https://apt.postgresql.org/pub/repos/apt/pool/main/p/postgresql-14/

The packages should be accessible via the 'focal-pgdg-testing'
distribution, where they initially land before eventual promotion to the
'focal-pgdg' distribution.
https://apt.postgresql.org/pub/repos/apt/dists/focal-pgdg-testing/main/binary-arm64/

As for Canonical, my initial guess is to file a bug / question on Launchpad
for the postgresql-12 release (which is current for Ubuntu Focal), or
connect with their developers on the
[email-masked] mailing
list.
https://launchpad.net/ubuntu/+source/postgresql-12

Aaron

---

From: "Pop, Sebastian" <[email-masked]>
Date: Thu, 18 Nov 2021 22:56:48 +0000

> The packages should be accessible via the 'focal-pgdg-testing' distribution, where they initially land before eventual promotion to the 'focal-pgdg' distribution.

Thanks Aaron for the explanation.

> As for Canonical, my initial guess is to file a bug / question on Launchpad for the postgresql-12 release (which is current for Ubuntu Focal), or connect with their developers on the [email-masked]<mailto:[email-masked]> mailing list.

I am in touch with Canonical to update posgres packages in Focal with outline-atomics.

Sebastian

---

From: Christoph Berg <[email-masked]>
Date: Fri, 19 Nov 2021 17:23:17 +0100

Hi,

Re: Pop, Sebastian
> https://pgdgbuild.dus.dg-i.net/view/PostgreSQL/job/postgresql-15-binaries/architecture=amd64,distribution=focal/345/

That's amd64.

https://salsa.debian.org/postgresql/postgresql/-/blob/15/debian/patches/focal-arm64-outline-atomics

> Does Canonical rely on these builds to update the Focal PostgreSQL packages?

I don't know what Ubuntu's policy on updating released packages is.

Christoph

---

From: "Pop, Sebastian" <[email-masked]>
Date: Fri, 19 Nov 2021 16:30:20 +0000

> https://salsa.debian.org/postgresql/postgresql/-/blob/15/debian/patches/focal-arm64-outline-atomics

Sorry about that typo.  Arm64 branch 15 looks good.  Thanks Christoph.

> I don't know what Ubuntu's policy on updating released packages is.

I took the issue to Canonical and I will follow up with them to update the PostgreSQL packages.

Thanks,
Sebastian