Neil Conway <nconway@klamath.dyndns.org> writes:
> The attached patch re-implements BufferGetBlockNumber() as a macro,
> for performance reasons.
The trouble with this is that it forces storage/buf_internals.h to
become part of bufmgr.h, thereby completely destroying any information
hiding that we had from the division of the two headers --- any client
of bufmgr.h might now access bufmgr internal stuff without noticing
that it was doing wrong.
I am not convinced that BufferGetBlockNumber is a bottleneck anyway;
at least not if you don't have Assert checking enabled.
There are a few places that do things like
ItemPointerSet(&(tuple->t_self), BufferGetBlockNumber(buffer), offnum);
which I believe results in two calls to BufferGetBlockNumber because of
the way BlockIdSet is coded. This would be good to clean up, but it's
BlockIdSet's problem not BufferGetBlockNumber's.
> It also adds an assertion that should probably
> be present.
FWIW, BufferIsPinned also checks BufferIsValid; you do not need both.
regards, tom lane
