On Mon, Jan 18, 2016 at 4:24 PM, Peter Geoghegan <pg@heroku.com> wrote:
> On Mon, Jan 18, 2016 at 2:14 PM, Kevin Grittner <kgrittn@gmail.com> wrote:
>> It's hard to understand quite what you're saying there. If you're
>> saying that code changes that should be performance neutral can
>> sometimes affect performance because of alignment of code with
>> cache line boundaries -- I absolutely agree; is that an argument
>> against performance testing performance patches?
>
> No, it isn't an argument against performance testing patches like
> this, but I don't think anyone suggested otherwise.
I'm still not sure what argument he *was* making, but I'm glad to
hear it wasn't that.
> Of course every performance related patch should be tested to
> make sure it meets its goals and at acceptable cost,
I argued that it should be tested to ensure that it caused no
regression in a case which has been a problem for some of our
customers (spinlock contention at high concurrency on a machine
with 4 or more NUMA nodes). We have been able to solve those
problems, but it has been a fussy business -- sometimes we have
tweaked spinlock code and sometimes that has not worked but an OS
upgrade has. I am quite unconvinced about whether the change will
help, hurt, or have no impact on these problems; I'm only arguing
for testing to find out.
> but I don't think that Andreas' patch is necessarily a
> performance patch. There can be value in removing superfluous
> code; doing so sometimes clarifies intent and understanding.
Well, that's why I said I would be satisfied with a neutral
benchmark result -- when there is a tie, the shorter, simpler code
should generally win. I'm not really sure what there was in what I
said to argue about; since that I've just been trying figure that
out. If we all agree, let's let it drop.
--
Kevin Grittner
EDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
