On Sun, Feb 21, 2016 at 2:28 PM, Robert Haas <robertmh...@gmail.com> wrote:
> On Sun, Feb 21, 2016 at 12:24 PM, Amit Kapila <amit.kapil...@gmail.com> > wrote: > >> On Sun, Feb 21, 2016 at 12:02 PM, Robert Haas <robertmh...@gmail.com> >> wrote: >> >>> On Sun, Feb 21, 2016 at 10:27 AM, Amit Kapila <amit.kapil...@gmail.com> >>> wrote: >>> >>>> >>>> Client_Count/Patch_Ver 1 64 128 256 >>>> HEAD(481725c0) 963 28145 28593 26447 >>>> Patch-1 938 28152 31703 29402 >>>> >>>> >>>> We can see 10~11% performance improvement as observed >>>> previously. You might see 0.02% performance difference with >>>> patch as regression, but that is just a run-to-run variation. >>>> >>> >>> Don't the single-client numbers show about a 3% regresssion? Surely not >>> 0.02%. >>> >> >> >> Sorry, you are right, it is ~2.66%, but in read-write pgbench tests, I >> could see such fluctuation. Also patch doesn't change single-client >> case. However, if you still feel that there could be impact by patch, >> I can re-run the single client case once again with different combinations >> like first with HEAD and then patch and vice versa. >> > > Are these results from a single run, or median-of-three? > > This was median-of-three, but the highest tps with patch is 1119 and with HEAD, it is 969 which shows a gain at single client-count. Sometimes, I see such differences, it could be due to auto vacuum getting triggered at some situations which lead to such variations. However, if I try 2-3 times, the difference generally gets disappeared unless there is some real regression or if just switch off auto vacuum and do manual vacuum after each run. This time, I haven't run the tests multiple times. > I mean, my basic feeling is that I would not accept a 2-3% regression in > the single client case to get a 10% speedup in the case where we have 128 > clients. > I understand your point. I think to verify whether it is run-to-run variation or an actual regression, I will re-run these tests on single client multiple times and post the result. > A lot of people will not have 128 clients; quite a few will have a > single session, or just a few. Sometimes just making the code more complex > can hurt performance in subtle ways, e.g. by making it fit into the L1 > instruction cache less well. If the numbers you have here are accurate, > I'd vote to reject the patch. > > One point to note is that this patch along with first patch which I posted in this thread to increase clog buffers can make significant reduction in contention on CLogControlLock. OTOH, I think introducing regression at single-client is also not a sane thing to do, so lets first try to find if there is actually any regression and if it is, can we mitigate it by writing code with somewhat fewer instructions or in a slightly different way and then we can decide whether it is good to reject the patch or not. Does that sound reasonable to you? With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
