Hi,
Currently lwlock acquisition, whether direct, or via the LockBuffer()
wrapper, have a mode argument. I don't like that much, for three
reasons:
1) I've tried to add annotations for static analyzers to help with
locking correctness. The ones I looked at don't support annotating
shared/exclusive locks where the mode is specified as a variable.
2) When doing performance analysis it's quite useful to be able to see
the difference between exlusive and shared acquisition. Typically all
one has access to is the symbol name though.
3) I don't like having the unnecessary branches for the lock mode, after
all a lot of the lock protected code is fairly hot. It's pretty
unnecessary because the caller almost (?) always uses a static lock
mode.
Therefore I'd like to replace the current lock functions with ones where
the lock mode is specified as part of the function name rather than an
argument.
To avoid unnecessary backward compat pains it seems best to first
introduce compat wrappers using the current signature, and then
subsequently replace in-core callers with the direct calls.
There's several harder calls though:
1) All of the above would benefit from lock release also being annotated
with the lock mode. That'd be a lot more invasive however. I think
it'd be best to add explicit functions (which would just assert
held_lwlocks[] being correct), but keep a wrapper that determines the
current lock level using held_lwlocks.
2) For performance it'd be nice if we could move the BufferIsLocal()
checks for LockBuffer* into the caller. Even better would be if we
made them inline wrappers around
LWLockAcquire(Shared|Exclusive). However, as the latter would require
making BufferDescriptorGetContentLock() available in bufmgr.h I think
that's not worth it. So I think we'd be best off having
LockBufferExclusive() be a static inline wrapper doing the
BufferIsLocal() check and then calling LockBufferExclusiveImpl
which'd do the LWLockAcquireExclusive().
Thoughts?
Greetings,
Andres Freund