Re: bufmgr code cleanup

Neil Conway <neilc(at)samurai(dot)com> writes:
> On Mon, 2003-11-03 at 10:00, Tom Lane wrote:
>> I do not actually agree with the "UnlockAndReleaseBuffer" changes
>> anyway. I think this obscures the code by making resource grabbing
>> and resource releasing code unsymmetrical

> Hmm... fair enough, I see your point. In that case, should I remove the
> UnlockAndReleaseBuffer macro and change all the places that use it to
> just do a LockBuffer() followed by ReleaseBuffer()?

Didn't realize we had one, actually. Wonder when it was put in? I
agree with removing it --- seems like it obscures the code to little
purpose.

>> not to mention incompatible
>> with code branches where the unlock and the buffer release can't be
>> merged because other things are done in between.

> This makes no sense to me: the macro is inapplicable in this case, but I
> don't see how this makes anything "incompatible". Can you elaborate?

Incompatible in the sense that it looks different. If you see in one
place

foo = ReadBuffer(...);
do some stuff;
LockBuffer(foo, LOCK);
do some stuff;
LockBuffer(foo, UNLOCK);
do some stuff;
ReleaseBuffer(foo);

then it's reasonably apparent that the LockBuffers pair together and so
do ReadBuffer/ReleaseBuffer. If other places replace this pattern by

foo = ReadBuffer(...);
do some stuff;
LockBuffer(foo, LOCK);
do some stuff;
UnLockAndReleaseBuffer(foo);

it looks visually different and the pairing of operations is lost.
ISTM this adds potential for confusion without really buying anything.

One could argue that LockBuffer(foo, UNLOCK) ought to be written
UnLockBuffer(foo) for symmetry with ReleaseBuffer. I'm not sure if
there are any places that really depend on the way it's coded now...

>> As for removing the BM_TRACE code, what's broken about it?

> Well, it doesn't compile, and probably hasn't for years, so I took that
> as a sign that there wasn't much interest in it.

Ah. Fair point I guess ...

regards, tom lane