On Fri, Jan 27, 2012 at 8:35 AM, Heikki Linnakangas
<heikki.linnakangas@enterprisedb.com> wrote:
>> I think you should break this off into a new function,
>> LWLockWaitUntilFree(), rather than treating it as a new LWLockMode.
>> Also, instead of adding lwWaitOnly, I would suggest that we generalize
>> lwWaiting and lwExclusive into a field lwWaitRequest, which can be set
>> to 1 for exclusive, 2 for shared, 3 for wait-for-free, etc. That way
>> we don't have to add another boolean every time someone invents a new
>> type of wait - not that that should hopefully happen very often. A
>> side benefit of this is that you can simplify the test in
>> LWLockRelease(): keep releasing waiters until you come to an exclusive
>> waiter, then stop. This possibly saves one shared memory fetch and
>> subsequent test instruction, which might not be trivial - all of this
>> code is extremely hot.
>
> Makes sense. Attached is a new version, doing exactly that.
I think I recommended a bad name for this function. It's really
LWLockAcquireOrWaitUntilFree. Can we rename that?
We might also want to consider what all the behaviors are that we
might want here. It strikes me that there are two possible actions
when a lock is immediately available (acquire or noop) and three
possible actions when it isn't (wait-then-acquire, wait-then-noop, or
immediate-noop), for a total of six possible combinations:
acquire/wait-then-acquire = LWLockAcquire
acquire/wait-then-noop = what you just added
acquire/immediate-noop = LWLockConditionalAcquire
noop/wait-then-acquire
noop/wait-then-noop
noop/immediate-noop
The last one is clearly pointless, since you don't need a lock at all
to do nothing. But is there a use case for either of the other two?
I can't immediately think of any reason why we'd want
noop/wait-then-acquire but noop/wait-then-noop seems potentially
useful (that's what I originally thought LWLockWaitUntilFree was going
to do). For example, if for some reason you wanted all the WAL
flushes to happen in one process, you could have everyone else use
that primitive to sleep, and then recheck whether enough flushing had
happened (I don't actually think that's better; it's not very robust
against WAL writer going away). Or, for ProcArrayLock, you might (as
you proposed previously) we could mark our XID with a removal sequence
number in lieu of actually removing it, and then wait for
ProcArrayLock to be free before overwriting it with a new XID. Since
we're out of time for 9.2 I don't think we probably want to devote too
much time to experimenting with this right now, but it might be
interesting to play around with it next cycle.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
