Re: Windows warnings from VS 2017

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Andres Freund <andres(at)anarazel(dot)de>
Cc: Andrew Dunstan <andrew(dot)dunstan(at)2ndquadrant(dot)com>, Haribabu Kommi <kommi(dot)haribabu(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Windows warnings from VS 2017
Date: 2017-10-11 22:20:24
Message-ID: 5100.1507760424@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Andres Freund <andres(at)anarazel(dot)de> writes:
> Andrew Dunstan <andrew(dot)dunstan(at)2ndquadrant(dot)com> writes:
>> I presume if you can make that assertion you already have something
>> along those lines?

> Not really. I just replaced memset with MemSetAligned in a bunch of
> places in the code and looked at profiles.

Well, it's not that much work to try it and see. I compared results
of this simplistic test case:
pgbench -S -c 1 -T 60 bench
(using a scale-factor-10 pgbench database) on current HEAD and HEAD
with the attached patch, which just lobotomizes all the MemSet
macros into memset(). Median of 3 runs is 9698 tps for HEAD and
9675 tps with the patch; the range is ~100 tps though, making
this difference well within the noise level.

I did this using RHEL6's gcc (Red Hat 4.4.7-18), which is pretty
far from bleeding-edge so I think it's okay as a baseline for
what optimizations we can expect to find used in the field.

So I can't sustain Andres' assertion that memset is actually faster
for the cases we care about, but it certainly doesn't seem any
slower either. It would be interesting to check compromise
patches, such as keeping only the MemSetLoop case.

BTW, I got a couple of warnings with this patch:

pmsignal.c: In function 'PMSignalShmemInit':
pmsignal.c:104: warning: passing argument 1 of 'memset' discards qualifiers from pointer target type
/usr/include/string.h:65: note: expected 'void *' but argument is of type 'volatile struct PMSignalData *'
procsignal.c: In function 'ProcSignalInit':
procsignal.c:119: warning: passing argument 1 of 'memset' discards qualifiers from pointer target type
/usr/include/string.h:65: note: expected 'void *' but argument is of type 'volatile sig_atomic_t *'

Those seem like maybe something to look into. MemSet is evidently
casting away pointer properties that it perhaps shouldn't.

regards, tom lane

Attachment Content-Type Size
no-more-MemSet.patch text/x-diff 2.9 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Lukas Fittl 2017-10-11 22:23:07 Re: [HACKERS] Re: BUG #14821: idle_in_transaction_session_timeout sometimes gets ignored when statement timeout is pending
Previous Message Andres Freund 2017-10-11 22:04:02 Re: SendRowDescriptionMessage() is slow for queries with a lot of columns