Date: Tue, 17 Oct 2017 13:17:10 -0700
From: Andres Freund <andres@anarazel.de>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Andrew Dunstan <andrew.dunstan@2ndquadrant.com>,
	Haribabu Kommi <kommi.haribabu@gmail.com>,
	PostgreSQL-development <pgsql-hackers@postgresql.org>
Subject: Re: Windows warnings from VS 2017
Message-ID: <20171017201710.xsgresmef7brwwik@alap3.anarazel.de>
References: <8649.1505951694@sss.pgh.pa.us>
 <acf6b500-0f91-cb38-d76b-17b6de29a606@2ndQuadrant.com>
 <c8fc2cd2-584f-1bf7-8d4a-ae05db709ad2@2ndQuadrant.com>
 <CAJrrPGfJMmM3iSrxAqiSJfV8L=3KBQobK9Nhbsns2jGD+fQ==w@mail.gmail.com>
 <bfa18917-9cfa-57ce-1806-043d478c9519@2ndQuadrant.com>
 <7214.1506000631@sss.pgh.pa.us>
 <20170922014108.64om7wh3ozfhcczl@alap3.anarazel.de>
 <c7e5532b-5db0-cab5-cbec-0c8c80b1035e@2ndQuadrant.com>
 <20171011212816.vbpff7jkolna6kbm@alap3.anarazel.de>
 <5100.1507760424@sss.pgh.pa.us>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <5100.1507760424@sss.pgh.pa.us>
Precedence: bulk
Sender: pgsql-hackers-owner@postgresql.org

On 2017-10-11 18:20:24 -0400, Tom Lane wrote:
> 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.

Well, that'd be because that workload doesn't exercise either version of
memset to a meaningful degree, right?

> +#define MemSetAligned(start, val, len) memset(start, val, len)

If we actually care about this optimization, which seems to solely serve
palloc0fast, we could tell the compiler about the guaranteed alignment
etc.  I'm doubtful it's worth making this work for palloc0fast alone,
but I think it might be worthwhile to make palloc0 an inline function
like

#ifdef YES_I_AM_A_GOOD_COMPILER
/* not needed here, but probably good for plenty other places */
#define pg_attribute_assume_aligned(align) __attribute__((assume_aligned(align)))
#define pg_assume_aligned(ptr, align)  __builtin_assume_aligned(ptr, align)
#else
...
#end

extern void *palloc(Size size) pg_attribute_assume_aligned(MAXALIGN);

static inline void *
palloc0(Size size)
{
    void *mem = palloc(size);

    memset(mem, 0, size);
    return mem;
}

that should allow the compiler to always optimize the memset based on
the alignment - at least x86 gcc does so - and on the size if a
built-time constant (nearly every compiler does so).

I assume we probably should do this dance not just for palloc, but for
the other allocation functions too.

Greetings,

Andres Freund


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers