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.
Perhaps you could also try it on a couple of different machines (e.g. MacBook Pro and a couple of different large servers).
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?
