Re: Spinlocks, yet again: analysis and proposed patches

I wrote:
> Another thought came to mind: maybe the current data layout for LWLocks
> is bad. Right now, the spinlock that protects each LWLock data struct
> is itself part of the struct, and since the structs aren't large (circa
> 20 bytes), the whole thing is usually all in the same cache line. ...
> Maybe it'd be better to allocate the spinlocks off by themselves.

Well, this is odd. I made up a patch to do this (attached) and found
that it pretty much sucks. Still the 4-way Opteron, previous best
(slock-no-cmpb and spin-delay-2):
1 31s 2 42s 4 51s 8 100s
with lwlock-separate added:
1 31s 2 52s 4 106s 8 213s

What I had expected to see was a penalty in the single-thread case (due
to instructions added to the LWLock functions), but there isn't any.
I did not expect to see a factor-of-2 penalty for four threads.

I guess what this means is that there's no real problem with losing the
cache line while manipulating the LWLock, which is what the patch was
intended to prevent. Instead, we're paying for swapping two cache lines
(the spinlock and the LWLock) across processors instead of just one line.
But that should at worst be a 2x inflation of the time previously spent
in LWLockAcquire/Release, which is surely not yet all of the application
;-). Why the heck is this so bad? Should we expect that apparently
minor changes in shared data structures might be costing equivalently
huge penalties in SMP performance elsewhere?

Unfortunately I don't have root on the Opteron and can't run oprofile.
But I'd really like to see some oprofile stats from these two cases
so we can figure out what in the world is going on here. Can anyone
help?

regards, tom lane