testing HS/SR - 1 vs 2 performance
in [PgSql]
|
Prev: pg_ctl stop -m immediate on the primary server inflatessequences
Next: [HACKERS] non-reproducible failure of random test on HEAD
From: Simon Riggs on 18 Apr 2010 07:01 On Sun, 2010-04-18 at 08:24 +0100, Simon Riggs wrote: > On Sat, 2010-04-17 at 18:52 -0400, Tom Lane wrote: > > Simon Riggs <simon(a)2ndQuadrant.com> writes: > > > What I'm not clear on is why you've used a spinlock everywhere when only > > > weak-memory thang CPUs are a problem. Why not have a weak-memory-protect > > > macro that does does nada when the hardware already protects us? (i.e. a > > > spinlock only for the hardware that needs it). > > > > Well, we could certainly consider that, if we had enough places where > > there was a demonstrable benefit from it. I couldn't measure any real > > slowdown from adding a spinlock in that sinval code, so I didn't propose > > doing so at the time --- and I'm pretty dubious that this code is > > sufficiently performance-critical to justify the work, either. > > OK, I'll put a spinlock around access to the head of the array. v2 patch attached -- Simon Riggs www.2ndQuadrant.com
From: David Fetter on 18 Apr 2010 16:16 On Sun, Apr 18, 2010 at 12:01:05PM +0100, Simon Riggs wrote: > On Sun, 2010-04-18 at 08:24 +0100, Simon Riggs wrote: > > On Sat, 2010-04-17 at 18:52 -0400, Tom Lane wrote: > > > Simon Riggs <simon(a)2ndQuadrant.com> writes: > > > > What I'm not clear on is why you've used a spinlock everywhere > > > > when only weak-memory thang CPUs are a problem. Why not have a > > > > weak-memory-protect macro that does does nada when the > > > > hardware already protects us? (i.e. a spinlock only for the > > > > hardware that needs it). > > > > > > Well, we could certainly consider that, if we had enough places > > > where there was a demonstrable benefit from it. I couldn't > > > measure any real slowdown from adding a spinlock in that sinval > > > code, so I didn't propose doing so at the time --- and I'm > > > pretty dubious that this code is sufficiently > > > performance-critical to justify the work, either. > > > > OK, I'll put a spinlock around access to the head of the array. > > v2 patch attached If you've committed this, or any other patch you've sent here, *please* mention so on the same thread. Cheers, David. -- David Fetter <david(a)fetter.org> http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fetter(a)gmail.com iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Simon Riggs on 18 Apr 2010 16:22 On Sun, 2010-04-18 at 13:16 -0700, David Fetter wrote: > On Sun, Apr 18, 2010 at 12:01:05PM +0100, Simon Riggs wrote: > > On Sun, 2010-04-18 at 08:24 +0100, Simon Riggs wrote: > > > On Sat, 2010-04-17 at 18:52 -0400, Tom Lane wrote: > > > > Simon Riggs <simon(a)2ndQuadrant.com> writes: > > > > > What I'm not clear on is why you've used a spinlock everywhere > > > > > when only weak-memory thang CPUs are a problem. Why not have a > > > > > weak-memory-protect macro that does does nada when the > > > > > hardware already protects us? (i.e. a spinlock only for the > > > > > hardware that needs it). > > > > > > > > Well, we could certainly consider that, if we had enough places > > > > where there was a demonstrable benefit from it. I couldn't > > > > measure any real slowdown from adding a spinlock in that sinval > > > > code, so I didn't propose doing so at the time --- and I'm > > > > pretty dubious that this code is sufficiently > > > > performance-critical to justify the work, either. > > > > > > OK, I'll put a spinlock around access to the head of the array. > > > > v2 patch attached > > If you've committed this, or any other patch you've sent here, > *please* mention so on the same thread. I haven't yet. I've written two patches - this is a major module rewrite and is still under discussion. The other patch has nothing to do with this (though I did accidentally include a couple of changes from this patch and immediately revoked them). I will wait awhile to see if anybody has some independent test results. -- Simon Riggs www.2ndQuadrant.com -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: David Fetter on 18 Apr 2010 16:23 On Sun, Apr 18, 2010 at 09:22:21PM +0100, Simon Riggs wrote: > On Sun, 2010-04-18 at 13:16 -0700, David Fetter wrote: > > On Sun, Apr 18, 2010 at 12:01:05PM +0100, Simon Riggs wrote: > > > On Sun, 2010-04-18 at 08:24 +0100, Simon Riggs wrote: > > > > On Sat, 2010-04-17 at 18:52 -0400, Tom Lane wrote: > > > > > Simon Riggs <simon(a)2ndQuadrant.com> writes: > > > > > > What I'm not clear on is why you've used a spinlock > > > > > > everywhere when only weak-memory thang CPUs are a problem. > > > > > > Why not have a weak-memory-protect macro that does does > > > > > > nada when the hardware already protects us? (i.e. a > > > > > > spinlock only for the hardware that needs it). > > > > > > > > > > Well, we could certainly consider that, if we had enough > > > > > places where there was a demonstrable benefit from it. I > > > > > couldn't measure any real slowdown from adding a spinlock in > > > > > that sinval code, so I didn't propose doing so at the time > > > > > --- and I'm pretty dubious that this code is sufficiently > > > > > performance-critical to justify the work, either. > > > > > > > > OK, I'll put a spinlock around access to the head of the > > > > array. > > > > > > v2 patch attached > > > > If you've committed this, or any other patch you've sent here, > > *please* mention so on the same thread. > > I haven't yet. I've written two patches - this is a major module > rewrite and is still under discussion. The other patch has nothing > to do with this (though I did accidentally include a couple of > changes from this patch and immediately revoked them). > > I will wait awhile to see if anybody has some independent test > results. Thanks :) Cheers, David. -- David Fetter <david(a)fetter.org> http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fetter(a)gmail.com iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Robert Haas on 20 Apr 2010 07:50
On Sun, Apr 18, 2010 at 4:22 PM, Simon Riggs <simon(a)2ndquadrant.com> wrote: > On Sun, 2010-04-18 at 13:16 -0700, David Fetter wrote: >> On Sun, Apr 18, 2010 at 12:01:05PM +0100, Simon Riggs wrote: >> > On Sun, 2010-04-18 at 08:24 +0100, Simon Riggs wrote: >> > > On Sat, 2010-04-17 at 18:52 -0400, Tom Lane wrote: >> > > > Simon Riggs <simon(a)2ndQuadrant.com> writes: >> > > > > What I'm not clear on is why you've used a spinlock everywhere >> > > > > when only weak-memory thang CPUs are a problem. Why not have a >> > > > > weak-memory-protect macro that does does nada when the >> > > > > hardware already protects us? (i.e. a spinlock only for the >> > > > > hardware that needs it). >> > > > >> > > > Well, we could certainly consider that, if we had enough places >> > > > where there was a demonstrable benefit from it. I couldn't >> > > > measure any real slowdown from adding a spinlock in that sinval >> > > > code, so I didn't propose doing so at the time --- and I'm >> > > > pretty dubious that this code is sufficiently >> > > > performance-critical to justify the work, either. >> > > >> > > OK, I'll put a spinlock around access to the head of the array. >> > >> > v2 patch attached >> >> If you've committed this, or any other patch you've sent here, >> *please* mention so on the same thread. > > I haven't yet. I've written two patches - this is a major module rewrite > and is still under discussion. The other patch has nothing to do with > this (though I did accidentally include a couple of changes from this > patch and immediately revoked them). > > I will wait awhile to see if anybody has some independent test results. So, does anyone have a few cycles to test this out? We are down to handful of remaining open items, so getting this tested and committed sooner = beta sooner. ....Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers |