Horton

Thread

Commits

  1. Force "restrict" not to be used when compiling with xlc.

  2. Rely on sizeof(typename) rather than sizeof(variable) in pqformat.h.

  3. Use C99 restrict via pg_restrict, rather than restrict directly.

  4. Improve performance of SendRowDescriptionMessage.

  5. Partially flatten struct tupleDesc so that it can be used in DSM.

  6. Remove useless duplicate inclusions of system header files.

  1. pgsql: Improve performance of SendRowDescriptionMessage.

    Andres Freund <andres@anarazel.de> — 2017-10-12T00:28:21Z

    Improve performance of SendRowDescriptionMessage.

There's three categories of changes leading to better performance:
- Splitting the per-attribute part of SendRowDescriptionMessage into a
  v2 and a v3 version allows avoiding branches for every attribute.
- Preallocating the size of the buffer to be big enough for all
  attributes and then using pq_write* avoids unnecessary buffer
  size checks & resizing.
- Reusing a persistently allocated StringInfo for all
  SendRowDescriptionMessage() invocations avoids repeated allocations
  & reallocations.

Author: Andres Freund
Discussion: https://postgr.es/m/20170914063418.sckdzgjfrsbekae4@alap3.anarazel.de

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/4c119fbcd49ba882791c7b99a1e934b985468e9f

Modified Files
--------------
src/backend/access/common/printtup.c | 146 ++++++++++++++++++++++++++---------
src/backend/tcop/postgres.c          |  35 +++++++--
src/include/access/printtup.h        |   4 +-
3 files changed, 138 insertions(+), 47 deletions(-)

  2. Re: pgsql: Improve performance of SendRowDescriptionMessage.

    Tom Lane <tgl@sss.pgh.pa.us> — 2017-10-12T14:44:58Z

    Andres Freund <andres@anarazel.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.

			regards, tom lane

  3. Re: pgsql: Improve performance of SendRowDescriptionMessage.

    Andres Freund <andres@anarazel.de> — 2017-10-12T23:08:44Z

    On 2017-10-12 10:44:58 -0400, Tom Lane wrote:
> Andres Freund <andres@anarazel.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

  4. Re: pgsql: Improve performance of SendRowDescriptionMessage.

    Andres Freund <andres@anarazel.de> — 2017-10-12T23:48:29Z

    On 2017-10-12 16:08:44 -0700, Andres Freund wrote:
> 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>

Hm, it got removed as part of
http://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=9e3755ecb2d058f7d123dd35a2e1784006190962
but that's not an explanation, because
c.h includes sys/types.h. Which according to IBM's docs
https://www.ibm.com/support/knowledgecenter/en/ssw_aix_71/com.ibm.aix.basetrf1/getpeereid.htm
is the right thing to include.  Given that xlc doesn't complain, I'll
just assume this is some issue with the headers gcc uses on aix, but I'm
far from confident.

Greetings,

Andres Freund

  5. Re: pgsql: Improve performance of SendRowDescriptionMessage.

    Tom Lane <tgl@sss.pgh.pa.us> — 2017-10-13T00:06:32Z

    Andres Freund <andres@anarazel.de> writes:
>> fe-connect.c:2382:6: warning: implicit declaration of function 'getpeereid' [-Wimplicit-function-declaration]
>> Looks like we're missing
>> #include <sys/types.h>

> Hm, it got removed as part of
> http://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=9e3755ecb2d058f7d123dd35a2e1784006190962
> but that's not an explanation, because

Nope, because that's quite old.  The oldest make log on praxis for
sungazer is from 2015-08-31, and it contains these warning lines:

 auth.c:1585:2: warning: implicit declaration of function 'getpeereid' [-Wimplicit-function-declaration]
 ip.c:228:31: warning: large integer implicitly truncated to unsigned type [-Woverflow]
 pg_locale.c:1284:3: warning: implicit declaration of function 'wcstombs_l' [-Wimplicit-function-declaration]
 fe-connect.c:1991:6: warning: implicit declaration of function 'getpeereid' [-Wimplicit-function-declaration]
 ip.c:228:31: warning: large integer implicitly truncated to unsigned type [-Woverflow]

It looks like we have grown another occurrence of the "implicitly
truncated" nag, but otherwise it's the same warnings in the most
recent log.

			regards, tom lane

  6. Re: pgsql: Improve performance of SendRowDescriptionMessage.

    Andres Freund <andres@anarazel.de> — 2017-10-13T00:09:14Z

    On 2017-10-12 20:06:32 -0400, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> >> fe-connect.c:2382:6: warning: implicit declaration of function 'getpeereid' [-Wimplicit-function-declaration]
> >> Looks like we're missing
> >> #include <sys/types.h>
> 
> > Hm, it got removed as part of
> > http://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=9e3755ecb2d058f7d123dd35a2e1784006190962
> > but that's not an explanation, because
> 
> Nope, because that's quite old.

Right. I'd mentioned that it's *not* that commit, even though it
initially looked suspicious.

Greetings,

Andres Freund

  7. Re: pgsql: Improve performance of SendRowDescriptionMessage.

    Tom Lane <tgl@sss.pgh.pa.us> — 2017-10-13T00:24:23Z

    Andres Freund <andres@anarazel.de> writes:
> On 2017-10-12 20:06:32 -0400, Tom Lane wrote:
>> Nope, because that's quite old.

> Right. I'd mentioned that it's *not* that commit, even though it
> initially looked suspicious.

Right, my point was that nothing else we'd changed recently broke
this either.

			regards, tom lane

  8. Re: pgsql: Improve performance of SendRowDescriptionMessage.

    Noah Misch <noah@leadboat.com> — 2017-10-13T02:35:36Z

    On Thu, Oct 12, 2017 at 04:08:44PM -0700, Andres Freund wrote:
> 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?

I can confirm it allows "make check" to pass.  Specifically, I did this
against commit 91d5f1a:

--- src/include/pg_config.h~    2017-10-12 18:11:33.000000000 -0700
+++ src/include/pg_config.h     2017-10-12 18:22:34.000000000 -0700
@@ -929 +929 @@
- #define pg_restrict __restrict
+ #define pg_restrict
@@ -934 +934 @@
- #define restrict __restrict
+ #define restrict

I have no reason to believe this is specific to hornet's installation, so I
recommend against altering hornet's configuration.  It's too likely that the
next xlc user will need to do the same thing.  I don't see the problem with
xlc 13.1.3, though.

> Otherwise I can push a platform fix that disables it.

This sounds reasonable.

nm

  9. Re: pgsql: Improve performance of SendRowDescriptionMessage.

    Noah Misch <noah@leadboat.com> — 2017-10-13T02:39:04Z

    On Thu, Oct 12, 2017 at 04:48:29PM -0700, Andres Freund wrote:
> On 2017-10-12 16:08:44 -0700, Andres Freund wrote:
> > 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>
> 
> Hm, it got removed as part of
> http://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=9e3755ecb2d058f7d123dd35a2e1784006190962
> but that's not an explanation, because
> c.h includes sys/types.h. Which according to IBM's docs
> https://www.ibm.com/support/knowledgecenter/en/ssw_aix_71/com.ibm.aix.basetrf1/getpeereid.htm
> is the right thing to include.  Given that xlc doesn't complain, I'll
> just assume this is some issue with the headers gcc uses on aix, but I'm
> far from confident.

The relevant xlc warning is disabled by default.  "xlc -qinfo=pro" does
complain.  <sys/types.h> provides no such prototype.  <sys/socket.h> provides
a getpeereid() prototype, but under C++ only.  (For getpeername(), it provides
both C and C++ prototypes.)  Thus, the AIX documentation is wrong, and
/usr/include is buggy.

  10. Re: pgsql: Improve performance of SendRowDescriptionMessage.

    Tom Lane <tgl@sss.pgh.pa.us> — 2017-10-13T02:44:20Z

    Noah Misch <noah@leadboat.com> writes:
> On Thu, Oct 12, 2017 at 04:08:44PM -0700, Andres Freund wrote:
>> Noah, any chance you could force restrict to off on that animal?

> I can confirm it allows "make check" to pass.

So that leaves us with two theories:

1. hornet's compiler contains a bug that causes it to misoptimize
in the presence of "restrict".

2. There's a bug in the way HEAD is applying "restrict", which happens
not to manifest on other platforms.

While I have to agree with Andres' evident feeling that it's probably
#1, I do not think we should dismiss #2 without inquiring a bit
harder.  It would be really useful, I think, if we could characterize
exactly how the RowDescription output is broken in that build.
Noah, could you capture some of those messages somehow?

			regards, tom lane

  11. Re: pgsql: Improve performance of SendRowDescriptionMessage.

    Noah Misch <noah@leadboat.com> — 2017-10-13T07:02:47Z

    In an earlier message, I said I didn't see the problem with xlc 13.1.3.  I
withdraw that statement.  I had tested old code (commit c629324, 20 Aug),
rendering the test invalid.  xlc 13.1.3 does break commit 91d5f1a, and
removing "restrict" fixes things as it did the older version.

On Thu, Oct 12, 2017 at 10:44:20PM -0400, Tom Lane wrote:
> Noah Misch <noah@leadboat.com> writes:
> > On Thu, Oct 12, 2017 at 04:08:44PM -0700, Andres Freund wrote:
> >> Noah, any chance you could force restrict to off on that animal?
> 
> > I can confirm it allows "make check" to pass.
> 
> So that leaves us with two theories:
> 
> 1. hornet's compiler contains a bug that causes it to misoptimize
> in the presence of "restrict".
> 
> 2. There's a bug in the way HEAD is applying "restrict", which happens
> not to manifest on other platforms.
> 
> While I have to agree with Andres' evident feeling that it's probably
> #1, I do not think we should dismiss #2 without inquiring a bit
> harder.  It would be really useful, I think, if we could characterize
> exactly how the RowDescription output is broken in that build.
> Noah, could you capture some of those messages somehow?

I hacked psql to call PQtrace() and ran "psql -Xc 'select true'" in the
defective configuration and in a working x64 GNU/Linux configuration.  I've
attached both PQtrace() products.

  12. Re: pgsql: Improve performance of SendRowDescriptionMessage.

    Andres Freund <andres@anarazel.de> — 2017-10-13T07:32:43Z

    On 2017-10-12 19:35:36 -0700, Noah Misch wrote:
> On Thu, Oct 12, 2017 at 04:08:44PM -0700, Andres Freund wrote:
> > 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?
> 
> I can confirm it allows "make check" to pass.  Specifically, I did this
> against commit 91d5f1a:
> 
> --- src/include/pg_config.h~    2017-10-12 18:11:33.000000000 -0700
> +++ src/include/pg_config.h     2017-10-12 18:22:34.000000000 -0700
> @@ -929 +929 @@
> - #define pg_restrict __restrict
> + #define pg_restrict
> @@ -934 +934 @@
> - #define restrict __restrict
> + #define restrict
> 
> I have no reason to believe this is specific to hornet's installation, so I
> recommend against altering hornet's configuration.  It's too likely that the
> next xlc user will need to do the same thing.

Yea, I wasn't trying to propose that - I just thought it'd be easier to
narrow down with access to the machine than 6h cycle buildfarm
debugging.


> > Otherwise I can push a platform fix that disables it.
> 
> This sounds reasonable.

I'm not getting a great vibe about the aix/xlc quality in this thread :(


Greetings,

Andres Freund

  13. Re: pgsql: Improve performance of SendRowDescriptionMessage.

    Andres Freund <andres@anarazel.de> — 2017-10-13T07:39:14Z

    Hi,

On 2017-10-13 00:02:47 -0700, Noah Misch wrote:
> I hacked psql to call PQtrace() and ran "psql -Xc 'select true'" in the
> defective configuration and in a working x64 GNU/Linux configuration.  I've
> attached both PQtrace() products.

Thanks!


> To backend> Msg Q
> To backend> "select true"
> To backend> Msg complete, length 17
> From backend> T
> From backend (#4)> 17
> From backend (#2)> 1
> From backend> "bool"
> From backend (#4)> 1
> From backend (#2)> 0
> From backend (#4)> 1140850688
> From backend (#2)> 2816
> From backend (#4)> 16777216
> From backend (#2)> 372
> From backend> D
> From backend (#4)> 11
> From backend> C
> From backend (#4)> 13
> From backend> "SELECT 1"
> From backend> Z
> From backend (#4)> 5
> From backend> Z
> From backend (#4)> 5
> From backend> I
> To backend> Msg X
> To backend> Msg complete, length 5

> To backend> Msg Q
> To backend> "select true"
> To backend> Msg complete, length 17
> From backend> T
> From backend (#4)> 29
> From backend (#2)> 1
> From backend> "bool"
> From backend (#4)> 0
> From backend (#2)> 0
> From backend (#4)> 16
> From backend (#2)> 1
> From backend (#4)> -1
> From backend (#2)> 0
> From backend> D
> From backend (#4)> 11
> From backend (#2)> 1
> From backend (#4)> 1
> From backend (1)> t
> From backend> C
> From backend (#4)> 13
> From backend> "SELECT 1"
> From backend> Z
> From backend (#4)> 5
> From backend> Z
> From backend (#4)> 5
> From backend> I
> To backend> Msg X
> To backend> Msg complete, length 5

That's certainly quite weird. I can't immediately pinpoint where the bug
is. I initially thought that the StringInfo's lengths might be wrong,
but that doesn'tqutie seem to make sense. Looks a bit like there's just
garbless mess in there...  Will have another look when I don't have to
force my eyes to stay open.

Greetings,

Andres Freund

  14. Re: pgsql: Improve performance of SendRowDescriptionMessage.

    Tom Lane <tgl@sss.pgh.pa.us> — 2017-10-13T15:26:35Z

    Noah Misch <noah@leadboat.com> writes:
> I hacked psql to call PQtrace() and ran "psql -Xc 'select true'" in the
> defective configuration and in a working x64 GNU/Linux configuration.  I've
> attached both PQtrace() products.

Thanks.  It looks to me like the xlc build simply forgets to send some
of the T-message fields: the message length observed by the frontend
is too short, and the reported field values starting with "1140850688"
correspond to the actual contents of the subsequent D message, rather
than what should be there.

Studying the values that are returned, a plausible conclusion is that
in the sequence

		pq_writestring(buf, NameStr(att->attname));
		pq_writeint32(buf, resorigtbl);
		pq_writeint16(buf, resorigcol);
		pq_writeint32(buf, atttypid);
		pq_writeint16(buf, att->attlen);
		pq_writeint32(buf, atttypmod);
		pq_writeint16(buf, format);

the pq_writeint32 calls are somehow becoming no-ops.  That would explain
the message being exactly 12 bytes too short, and the 6 bytes that are
there match what the pq_writeint16 calls should send.

Looking at the pq_writeintN function definitions, I'm annoyed by the fact
that Andres seems to have decided whether to write sizeof(ni) or sizeof(i)
with the aid of a ouija board.  That should be consistent.  I'd go with
sizeof(ni) myself, since that's the object actually being memcpy'd.
It seems unlikely that that could explain this bug, but maybe somehow
sizeof() misbehaves for a parameter that's been inlined away?

Anyway, I will go make the sizeof() usages consistent, just to satisfy
my own uncontrollable neatnik-ism.  Assuming that hornet stays red,
which is probable, we should disable "restrict" for that compiler.

			regards, tom lane

  15. Re: [HACKERS] pgsql: Improve performance of SendRowDescriptionMessage.

    Tom Lane <tgl@sss.pgh.pa.us> — 2017-10-13T17:48:07Z

    I wrote:
> Anyway, I will go make the sizeof() usages consistent, just to satisfy
> my own uncontrollable neatnik-ism.  Assuming that hornet stays red,
> which is probable, we should disable "restrict" for that compiler.

As expected, that didn't fix it.  Andres, are you going to put in
a disable?  Do we know exactly what we want to test for that?

			regards, tom lane

  16. Re: [HACKERS] pgsql: Improve performance of SendRowDescriptionMessage.

    Andres Freund <andres@anarazel.de> — 2017-10-13T18:01:06Z

    On 2017-10-13 13:48:07 -0400, Tom Lane wrote:
> I wrote:
> > Anyway, I will go make the sizeof() usages consistent, just to satisfy
> > my own uncontrollable neatnik-ism.  Assuming that hornet stays red,
> > which is probable, we should disable "restrict" for that compiler.
> 
> As expected, that didn't fix it.  Andres, are you going to put in
> a disable?  Do we know exactly what we want to test for that?

A easiest way to do this would be to put something like
CPPFLAGS="$CPPFLAGS -Dpg_restrict" into the existing
if test "$GCC" != yes ; then
block in a/src/template/linux. But that'd probably result in "macro
redefined" warnings or somesuch.

So it'd probably better to introduce a FORCE_DISABLE_RESTRICT=yes, set
at the same place, that's then tested before running the relevant
configure check?

Greetings,

Andres Freund

  17. Re: [HACKERS] pgsql: Improve performance of SendRowDescriptionMessage.

    Tom Lane <tgl@sss.pgh.pa.us> — 2017-10-13T18:19:22Z

    Andres Freund <andres@anarazel.de> writes:
> A easiest way to do this would be to put something like
> CPPFLAGS="$CPPFLAGS -Dpg_restrict" into the existing
> if test "$GCC" != yes ; then
> block in a/src/template/linux. But that'd probably result in "macro
> redefined" warnings or somesuch.

You mean src/template/aix, no?  Agreed, that seems like a reasonable
place to control it.  But I'm pretty sure the above would flat out
not work, the #define in pg_config.h would override it.

> So it'd probably better to introduce a FORCE_DISABLE_RESTRICT=yes, set
> at the same place, that's then tested before running the relevant
> configure check?

+1.  I think you don't actually have to skip the configure check,
and there might be some value in letting it carry on normally
(so that "restrict" is set properly).  We'd just want it to affect
what pg_restrict gets defined as.  Something like

if test "$ac_cv_c_restrict" = "no" -o "x$FORCE_DISABLE_RESTRICT" = "xyes"; then
  pg_restrict=""
else
  pg_restrict="$ac_cv_c_restrict"
fi

			regards, tom lane

  18. Re: [HACKERS] pgsql: Improve performance of SendRowDescriptionMessage.

    Andres Freund <andres@anarazel.de> — 2017-10-13T18:24:05Z

    On 2017-10-13 14:19:22 -0400, Tom Lane wrote:
> > So it'd probably better to introduce a FORCE_DISABLE_RESTRICT=yes, set
> > at the same place, that's then tested before running the relevant
> > configure check?
> 
> +1.  I think you don't actually have to skip the configure check,
> and there might be some value in letting it carry on normally
> (so that "restrict" is set properly).  We'd just want it to affect
> what pg_restrict gets defined as.  Something like

> if test "$ac_cv_c_restrict" = "no" -o "x$FORCE_DISABLE_RESTRICT" = "xyes"; then
>   pg_restrict=""
> else
>   pg_restrict="$ac_cv_c_restrict"

Yea, that works. Will make it so.

Greetings,

Andres Freund

  19. Re: [HACKERS] [COMMITTERS] pgsql: Improve performance of SendRowDescriptionMessage.

    Andres Freund <andres@anarazel.de> — 2022-08-13T19:08:43Z

    Hi Noah,

On 2017-10-13 11:24:05 -0700, Andres Freund wrote:
> On 2017-10-13 14:19:22 -0400, Tom Lane wrote:
> > > So it'd probably better to introduce a FORCE_DISABLE_RESTRICT=yes, set
> > > at the same place, that's then tested before running the relevant
> > > configure check?
> > 
> > +1.  I think you don't actually have to skip the configure check,
> > and there might be some value in letting it carry on normally
> > (so that "restrict" is set properly).  We'd just want it to affect
> > what pg_restrict gets defined as.  Something like
> 
> > if test "$ac_cv_c_restrict" = "no" -o "x$FORCE_DISABLE_RESTRICT" = "xyes"; then
> >   pg_restrict=""
> > else
> >   pg_restrict="$ac_cv_c_restrict"
> 
> Yea, that works. Will make it so.

Any chance you could check if this is still needed? I've a FIXME about it in
the meson code that I'd like to get rid of :)

Greetings,

Andres Freund

  20. Re: [HACKERS] [COMMITTERS] pgsql: Improve performance of SendRowDescriptionMessage.

    Noah Misch <noah@leadboat.com> — 2022-08-13T20:50:06Z

    On Sat, Aug 13, 2022 at 12:08:43PM -0700, Andres Freund wrote:
> On 2017-10-13 11:24:05 -0700, Andres Freund wrote:
> > On 2017-10-13 14:19:22 -0400, Tom Lane wrote:
> > > > So it'd probably better to introduce a FORCE_DISABLE_RESTRICT=yes, set
> > > > at the same place, that's then tested before running the relevant
> > > > configure check?
> > > 
> > > +1.  I think you don't actually have to skip the configure check,
> > > and there might be some value in letting it carry on normally
> > > (so that "restrict" is set properly).  We'd just want it to affect
> > > what pg_restrict gets defined as.  Something like
> > 
> > > if test "$ac_cv_c_restrict" = "no" -o "x$FORCE_DISABLE_RESTRICT" = "xyes"; then
> > >   pg_restrict=""
> > > else
> > >   pg_restrict="$ac_cv_c_restrict"
> > 
> > Yea, that works. Will make it so.
> 
> Any chance you could check if this is still needed? I've a FIXME about it in
> the meson code that I'd like to get rid of :)

I ran hornet against 1c497fa72d, the commit that failed in
https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=hornet&dt=2017-10-12%2022%3A14%3A41.
Similar failure today: "172 of 181 tests failed, 1 of these failures ignored."

  21. Re: [HACKERS] [COMMITTERS] pgsql: Improve performance of SendRowDescriptionMessage.

    Andres Freund <andres@anarazel.de> — 2022-08-14T00:48:47Z

    Hi,

On 2022-08-13 13:50:06 -0700, Noah Misch wrote:
> On Sat, Aug 13, 2022 at 12:08:43PM -0700, Andres Freund wrote:
> > Any chance you could check if this is still needed? I've a FIXME about it in
> > the meson code that I'd like to get rid of :)
> 
> I ran hornet against 1c497fa72d, the commit that failed in
> https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=hornet&dt=2017-10-12%2022%3A14%3A41.
> Similar failure today: "172 of 181 tests failed, 1 of these failures ignored."

Thanks. I guess we'll need the same logic then :/

Greetings,

Andres Freund