Re: [HACKERS] Re: Misaligned BufferDescriptors causing major performance problems on AMD
On Tue, Feb 4, 2014 at 4:24 PM, Peter Geoghegan wrote: > There is something you have not drawn explicit attention to that is > very interesting. If we take REL9_3_STABLE tip to be representative > (built with full -O2 optimization, no assertions just debugging > symbols), setting max_connections to 91 from 90 does not have the > effect of making the BufferDescriptors array aligned; it has the > effect of making it *misaligned*. I spoke too soon; the effect is indeed reversed on master (i.e. "bad" max_connection settings are misaligned, and vice-versa). -- Peter Geoghegan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: Misaligned BufferDescriptors causing major performance problems on AMD
On 2014-02-05 09:57:11 -0500, Tom Lane wrote: > Andres Freund writes: > > On 2014-02-04 16:24:02 -0800, Peter Geoghegan wrote: > >> I suspect that the scenario described in this article accounts for the > >> quite noticeable effect reported: http://danluu.com/3c-conflict > > > I don't think that's applicable here. > > Maybe, or maybe not, but I think it does say that we should be very wary > of proposals to force data structure alignment without any testing of the > consequences. I agree it needs testing, but what the page is talking about really, really doesn't have *anything* to do with this. What the author is talking about is not storing things *page* aligned (I.e. 4k or a multiple). The problem is that the beginning of each such page falls into the same cacheline set and thus doesn't utilize the entire L1/L2/L3 but only the single set they map into. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: Misaligned BufferDescriptors causing major performance problems on AMD
Andres Freund writes: > On 2014-02-04 16:24:02 -0800, Peter Geoghegan wrote: >> I suspect that the scenario described in this article accounts for the >> quite noticeable effect reported: http://danluu.com/3c-conflict > I don't think that's applicable here. Maybe, or maybe not, but I think it does say that we should be very wary of proposals to force data structure alignment without any testing of the consequences. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: Misaligned BufferDescriptors causing major performance problems on AMD
On 2014-02-04 16:24:02 -0800, Peter Geoghegan wrote: > On Mon, Feb 3, 2014 at 3:38 PM, Andres Freund wrote: > >> > A quick hack (attached) making BufferDescriptor 64byte aligned indeed > >> > restored performance across all max_connections settings. It's not > >> > surprising that a misaligned buffer descriptor causes problems - > >> > there'll be plenty of false sharing of the spinlocks otherwise. Curious > >> > that the the intel machine isn't hurt much by this. > > >> What fiddling are you thinking of? > > > > Basically always doing a TYPEALIGN(CACHELINE_SIZE, addr) before > > returning from ShmemAlloc() (and thereby ShmemInitStruct). > > There is something you have not drawn explicit attention to that is > very interesting. If we take REL9_3_STABLE tip to be representative > (built with full -O2 optimization, no assertions just debugging > symbols), setting max_connections to 91 from 90 does not have the > effect of making the BufferDescriptors array aligned; it has the > effect of making it *misaligned*. You reported that 91 was much better > than 90. I think that the problem actually occurs when the array *is* > aligned! I don't think you can learn much from the alignment in 9.3 vs. HEAD. Loads has changed since, most prominently and recently Robert's LWLock work. That certainly has changed allocation patterns. It will also depend on some other parameters, e.g. changing max_wal_senders, max_background_workers will also change the offset. It's not that 91 is intrinsically better, it just happened to give a aligned BufferDescriptors array when the other parameters weren't changed at the same time. > I suspect that the scenario described in this article accounts for the > quite noticeable effect reported: http://danluu.com/3c-conflict I don't think that's applicable here. What's described there is relevant for access patterns that are larger multiple of the cacheline size - but our's is exactly cacheline sized. What can happen in such scenarios is that all your accesses map to the same set of cachelines, so instead of using most of the cache, you end up using only 8 or so (8 is a common size of set associative caches these days). Theoretically we could see something like that for shared_buffers itself, but I *think* our accesses are too far spread around in them for that to be a significant issue. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: Misaligned BufferDescriptors causing major performance problems on AMD
On Mon, Feb 3, 2014 at 3:38 PM, Andres Freund wrote: >> > A quick hack (attached) making BufferDescriptor 64byte aligned indeed >> > restored performance across all max_connections settings. It's not >> > surprising that a misaligned buffer descriptor causes problems - >> > there'll be plenty of false sharing of the spinlocks otherwise. Curious >> > that the the intel machine isn't hurt much by this. >> What fiddling are you thinking of? > > Basically always doing a TYPEALIGN(CACHELINE_SIZE, addr) before > returning from ShmemAlloc() (and thereby ShmemInitStruct). There is something you have not drawn explicit attention to that is very interesting. If we take REL9_3_STABLE tip to be representative (built with full -O2 optimization, no assertions just debugging symbols), setting max_connections to 91 from 90 does not have the effect of making the BufferDescriptors array aligned; it has the effect of making it *misaligned*. You reported that 91 was much better than 90. I think that the problem actually occurs when the array *is* aligned! I suspect that the scenario described in this article accounts for the quite noticeable effect reported: http://danluu.com/3c-conflict -- Peter Geoghegan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: Misaligned BufferDescriptors causing major performance problems on AMD
On Tue, Feb 4, 2014 at 4:21 AM, Andres Freund wrote: > Which imo means fixing this got more important... I strongly agree. -- Peter Geoghegan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: Misaligned BufferDescriptors causing major performance problems on AMD
On 2014-02-04 00:38:19 +0100, Andres Freund wrote: > > > A quick hack (attached) making BufferDescriptor 64byte aligned indeed > > > restored performance across all max_connections settings. It's not > > > surprising that a misaligned buffer descriptor causes problems - > > > there'll be plenty of false sharing of the spinlocks otherwise. Curious > > > that the the intel machine isn't hurt much by this. > > > > I think that is explained here: > > > > http://www.agner.org/optimize/blog/read.php?i=142&v=t > > > > With Sandy Bridge, "Misaligned memory operands [are] handled efficiently". > > No, I don't think so. Those improvements afair refer to unaligned > accesses as in accessing a 4 byte variable at address % 4 != 0. So, Christian did some benchmarking on the intel machine, and his results were also lower than mine, and I've since confirmed that it's also possible to reproduce the alignment problems on the intel machine. Which imo means fixing this got more important... Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: Misaligned BufferDescriptors causing major performance problems on AMD
On 2014-02-03 15:17:13 -0800, Peter Geoghegan wrote:
> On Sun, Feb 2, 2014 at 7:13 AM, Andres Freund wrote:
> > Just as reference, we're talking about a performance degradation from
> > 475963.613865 tps to 197744.913556 in a pgbench -S -cj64 just by setting
> > max_connections to 90, from 91...
>
> That's pretty terrible.
Yea, I was scared myself.
> > A quick hack (attached) making BufferDescriptor 64byte aligned indeed
> > restored performance across all max_connections settings. It's not
> > surprising that a misaligned buffer descriptor causes problems -
> > there'll be plenty of false sharing of the spinlocks otherwise. Curious
> > that the the intel machine isn't hurt much by this.
>
> I think that is explained here:
>
> http://www.agner.org/optimize/blog/read.php?i=142&v=t
>
> With Sandy Bridge, "Misaligned memory operands [are] handled efficiently".
No, I don't think so. Those improvements afair refer to unaligned
accesses as in accessing a 4 byte variable at address % 4 != 0.
> > Now all this hinges on the fact that by a mere accident
> > BufferDescriptors are 64byte in size:
>
> Are they 64 bytes in size on REL9_*_STABLE? How about on win64? I
> think we're reasonably disciplined here already, but long is 32-bits
> in length even on win64. Looks like it would probably be okay, but as
> you say, it doesn't seem like something to leave to chance.
I haven't checked any branches yet, but I don't remember any recent
changes to sbufdesc. And I think we're lucky enough that all the used
types are the same width across common 64bit OSs.
But I absolutely agree we shouldn't leave this to chance.
> > We could polish up the attached patch and apply it to all the branches,
> > the costs of memory are minimal. But I wonder if we shouldn't instead
> > make ShmemInitStruct() always return cacheline aligned addresses. That
> > will require some fiddling, but it might be a good idea nonetheless?
> What fiddling are you thinking of?
Basically always doing a TYPEALIGN(CACHELINE_SIZE, addr) before
returning from ShmemAlloc() (and thereby ShmemInitStruct). After I'd
written the email I came across an interesting bit of code:
void *
ShmemAlloc(Size size)
{
...
/* extra alignment for large requests, since they are probably buffers
*/
if (size >= BLCKSZ)
newStart = BUFFERALIGN(newStart);
/*
* Preferred alignment for disk I/O buffers. On some CPUs, copies between
* user space and kernel space are significantly faster if the user buffer
* is aligned on a larger-than-MAXALIGN boundary. Ideally this should be
* a platform-dependent value, but for now we just hard-wire it.
*/
#define ALIGNOF_BUFFER 32
#define BUFFERALIGN(LEN)TYPEALIGN(ALIGNOF_BUFFER, (LEN))
So, we're already trying to make large allocations (which we'll be using
here) try to fit to a 32 byte alignment. Which unfortunately isn't
enough here, but it explains why max_connections has to be changed by
exactly 1 to achieve adequate performance...
So, the absolutely easiest thing would be to just change ALIGNOF_BUFFER
to 64. The current value was accurate in 2003 (common cacheline size
back then), but it really isn't anymore today. Anything relevant is
64byte.
But I really think we should also remove the BLCKSZ limit. There's
relatively few granular/dynamic usages of ShmemAlloc(), basically just
shared memory hashes. And I'd be rather unsurprised if aligning all
allocations for hashes resulted in a noticeable speedup. We have
frightening bottlenecks in those today.
> > I think we should also consider some more reliable measures to have
> > BufferDescriptors cacheline sized, rather than relying on the happy
> > accident. Debugging alignment issues isn't fun, too much of a guessing
> > game...
>
> +1. Maybe make code that isn't appropriately aligned fail to compile?
I was wondering about using some union trickery to make sure the array
only consists out of properly aligned types. That'd require some changes
but luckily code directly accessing the BufferDescriptors array isn't
too far spread.
Greetings,
Andres Freund
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
