Thread: make 'PrivateRefCount' 32 bits wide

This patch changes PrivateRefCount and LocalRefCount in the bufmgr from
being arrays of long to arrays of int32, per earlier discussion. This
should save a little memory on 64LP machines, and there's no way we need
to track > 2^31 references to a single buffer.

I also fixed the rather widespread assumption that a BufferDesc's
refcount is signed; it is not (and hence the correct printf formatting
code is %u, not %d).

I also changed ShowPinTrace to a bool, and removed some multi-line
string literals (yuck!)

Barring any objections, I intend to apply this patch within 24 hours.

-Neil

Neil Conway <neilc@samurai.com> writes:
> This patch changes PrivateRefCount and LocalRefCount in the bufmgr from
> being arrays of long to arrays of int32, per earlier discussion.

I just committed a bunch of changes in bufmgr --- hope I didn't tramp on
your toes too much.

> *** 176,184 ****
>       /*
>        * Allocate and zero local arrays of per-buffer info.
>        */
> !     BufferBlockPointers = (Block *) calloc(NBuffers, sizeof(Block));
> !     PrivateRefCount = (long *) calloc(NBuffers, sizeof(long));
> !     BufferLocks = (bits8 *) calloc(NBuffers, sizeof(bits8));

>       /*
>        * Convert shmem offsets into addresses as seen by this process. This
> --- 176,184 ----
>       /*
>        * Allocate and zero local arrays of per-buffer info.
>        */
> !     BufferBlockPointers = calloc(NBuffers, sizeof(*BufferBlockPointers));
> !     PrivateRefCount = calloc(NBuffers, sizeof(*PrivateRefCount));
> !     BufferLocks = calloc(NBuffers, sizeof(*BufferLocks));

This I think is a bad change.  The coding style
    ptr_var = (foo *) alloc(n * sizeof(foo));
is widely used in the backend, and I consider it good style because you
will get a warning if ptr_var is a pointer to something other than foo.
Removing the cast eliminates a significant protection against the risk
of using the wrong datatype in the sizeof calculation.  I realize that
if you mistakenly write
    ptr_var = (foo *) alloc(n * sizeof(bar));
then you're screwed anyway --- but this is a localized mistake.  Since
the declaration of ptr_var might be quite far from the allocation
statement, it's not hard to mess up ptr_var versus foo, whereas two type
references in the same statement are more likely to track each other.

As an example, your change to make PrivateRefCount be int32 * instead of
long * would draw no warning at all if the allocation command failed to
be changed to match, if the command were written in the cast-less way.

            regards, tom lane

On Wed, 2004-04-21 at 23:20, Tom Lane wrote:
> I just committed a bunch of changes in bufmgr --- hope I didn't tramp on
> your toes too much.

No problem, it didn't take too long to rediff. Besides, it was negligent
of me not to get that work committed sooner. (BTW, I'm finished exams
now, and I hope to get the rest of the bufmgr work wrapped up soon, as
well as the list rewrite.)

> This I think is a bad change.  The coding style
>     ptr_var = (foo *) alloc(n * sizeof(foo));
> is widely used in the backend, and I consider it good style [...]

Good point; I reverted this changed, rediffed the patch, and applied it.

-Neil