Re: pgsql: Improve performance of SendRowDescriptionMessage.

From: Andres Freund <andres(at)anarazel(dot)de>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Noah Misch <noah(at)leadboat(dot)com>
Cc: pgsql-committers(at)postgresql(dot)org
Subject: Re: pgsql: Improve performance of SendRowDescriptionMessage.
Date: 2017-10-12 23:08:44
Message-ID: 20171012230844.ye4tnye2shysvqpk@alap3.anarazel.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers pgsql-hackers

On 2017-10-12 10:44:58 -0400, Tom Lane wrote:
> Andres Freund <andres(at)anarazel(dot)de> writes:
> > Improve performance of SendRowDescriptionMessage.
>
> One or another of these patches has broken buildfarm member hornet.
> Apparently, it's transmitting incorrectly-formatted RowDescription
> messages.

This is curious.

Buildfarm animal hornet has failed. It's ppc64 compiled with xlc 12.1,
64 bit:
https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=hornet&dt=2017-10-12%2022%3A14%3A41

Buildfarm animal mandrill succeeded. It's ppc64 compiled with xlc 12.1,
32 bit:
https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=mandrill&dt=2017-10-12%2018%3A35%3A47

Buildfarm animal sungazer succeeded. It's ppc64 compiled with gcc 4.8,
64 bit:
https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=sungazer&dt=2017-10-12%2008%3A21%3A29

All of these are up to at least 31079a4.

So we've two animals (hornet, sungazer) that are:
#define SIZEOF_SIZE_T 8
#define WORDS_BIGENDIAN 1
#define restrict __restrict

one compiled with xlc that fails and one with gcc that succeeds. I'm
hesitant to reach for that, but I wonder if there's a compiler
bug. Alternatively there could be some undefined behaviour here that
only triggers on xlc 64bit, but I'm not quite seeing it.

Noah, any chance you could force restrict to off on that animal?
Otherwise I can push a platform fix that disables it.

Entirely independent of this, both machines report some interesting
warnings:
Hornet:
xlc_r -D_LARGE_FILES=1 -qnoansialias -g -O2 -qmaxmem=16384 -qsrcmsg -L../../src/port -L../../src/common -Wl,-blibpath:'/home/nm/farm/xlc64/HEAD/inst/lib:/usr/lib:/lib' -L../../src/port -lpgport -Wl,-bnoentry -Wl,-H512 -Wl,-bM:SRE -o timetravel.so timetravel.o -Wl,-bE:timetravel.exp -Wl,-bI:../../src/backend/postgres.imp
ld: 0711-224 WARNING: Duplicate symbol: .pg_strcasecmp
ld: 0711-224 WARNING: Duplicate symbol: .pg_ascii_tolower
ld: 0711-224 WARNING: Duplicate symbol: .pg_ascii_toupper
ld: 0711-224 WARNING: Duplicate symbol: .pg_tolower
ld: 0711-224 WARNING: Duplicate symbol: .pg_toupper
ld: 0711-224 WARNING: Duplicate symbol: .pg_strncasecmp

Hm. Seems we've some workaround in some platforms:
# src/template/win32

# --allow-multiple-definition is required to link pg_dump because it finds
# pg_toupper() etc. in both libpq and pgport

But that, uh, seems not good?

Sungazer:
wrap-gcc -D_THREAD_SAFE=1 -D_LARGE_FILES=1 -maix64 -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Wendif-labels -Wmissing-format-attribute -Wformat-security -fno-strict-aliasing -fwrapv -fexcess-precision=standard -g -O2 zic.o -L../../src/port -L../../src/common -Wl,-blibpath:'/home/nm/farm/gcc64/HEAD/inst/lib:/usr/lib:/lib' -lpgcommon -lpgport -lssl -lcrypto -lz -lreadline -lld -lm -o zic
ld: 0711-224 WARNING: Duplicate symbol: .bcopy
ld: 0711-224 WARNING: Duplicate symbol: .memmove

Hm.

wrap-gcc -D_THREAD_SAFE=1 -D_LARGE_FILES=1 -maix64 -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Wendif-labels -Wmissing-format-attribute -Wformat-security -fno-strict-aliasing -fwrapv -fexcess-precision=standard -g -O2 -I../../../src/include -c -o auth.o auth.c
auth.c: In function 'auth_peer':
auth.c:2002:2: warning: implicit declaration of function 'getpeereid' [-Wimplicit-function-declaration]
if (getpeereid(port->sock, &uid, &gid) != 0)
^
wrap-gcc -D_THREAD_SAFE=1 -D_LARGE_FILES=1 -maix64 -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Wendif-labels -Wmissing-format-attribute -Wformat-security -fno-strict-aliasing -fwrapv -fexcess-precision=standard -g -O2 -D_REENTRANT -D_THREAD_SAFE -D_POSIX_PTHREAD_SEMANTICS -DFRONTEND -DUNSAFE_STAT_OK -I. -I../../../src/include -I../../../src/port -I../../../src/port -DSO_MAJOR_VERSION=5 -c -o fe-connect.o fe-connect.c
fe-connect.c: In function 'PQconnectPoll':
fe-connect.c:2382:6: warning: implicit declaration of function 'getpeereid' [-Wimplicit-function-declaration]
if (getpeereid(conn->sock, &uid, &gid) != 0)
^

Looks like we're missing
#include <sys/types.h>

wrap-gcc -D_THREAD_SAFE=1 -D_LARGE_FILES=1 -maix64 -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Wendif-labels -Wmissing-format-attribute -Wformat-security -fno-strict-aliasing -fwrapv -fexcess-precision=standard -g -O2 -I../../../../src/include -c -o pg_locale.o pg_locale.c
pg_locale.c: In function 'wchar2char':
pg_locale.c:1648:3: warning: implicit declaration of function 'wcstombs_l' [-Wimplicit-function-declaration]
result = wcstombs_l(to, from, tolen, locale->info.lt);
^
This is curious. If I'm interpreting this correctly PGAC_FUNC_WCSTOMBS_L
fails to find a declaration, but AC_CHECK_FUNCS finds wcstombs_l, so we
happily use it.

Greetings,

Andres Freund

In response to

Responses

Browse pgsql-committers by date

  From Date Subject
Next Message Tom Lane 2017-10-12 23:14:17 Re: pgsql: Add configure infrastructure to detect support for C99's restric
Previous Message Andres Freund 2017-10-12 22:35:11 Re: pgsql: Add configure infrastructure to detect support for C99's restric

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2017-10-12 23:14:17 Re: pgsql: Add configure infrastructure to detect support for C99's restric
Previous Message Tom Lane 2017-10-12 23:08:19 Aggregate transition state merging vs. hypothetical set functions