Re: Question about coding of free space map

Lists: pgsql-hackers
From: Tatsuo Ishii <ishii(at)postgresql(dot)org>
To: pgsql-hackers(at)postgresql(dot)org
Subject: Question about coding of free space map
Date: 2014-08-26 02:13:23
Message-ID: 20140826.111323.42915740668569590.t-ishii@sraoss.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

While looking into backend/storage/freespace/freespace.c, I noticed
that struct FSMAddress is passed to functions by value, rather than
reference. I thought our code practice is defining pointer to a struct
data and using the pointer for parameter passing etc.

typedef struct RelationData *Relation;

IMO freespace.c is better to follow the practice.

Maybe this has been allowed because:

typedef struct
{
int level; /* level */
int logpageno; /* page number within the level */
} FSMAddress;

the struct size is 4+4=8 byte, which is same as 64 bit pointer. Still
I think it's better to use pointer to the struct because someday we
may want to add new member to the struct.

Best regards,
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese:http://www.sraoss.co.jp


From: Heikki Linnakangas <hlinnakangas(at)vmware(dot)com>
To: Tatsuo Ishii <ishii(at)postgresql(dot)org>, <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Question about coding of free space map
Date: 2014-08-29 12:14:12
Message-ID: 54006E94.3050007@vmware.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On 08/26/2014 05:13 AM, Tatsuo Ishii wrote:
> While looking into backend/storage/freespace/freespace.c, I noticed
> that struct FSMAddress is passed to functions by value, rather than
> reference. I thought our code practice is defining pointer to a struct
> data and using the pointer for parameter passing etc.
>
> typedef struct RelationData *Relation;
>
> IMO freespace.c is better to follow the practice.

There isn't really any strict coding rule on that. We pass RelFileNode's
by value in many functions, for example.

IMHO it's clearer to pass small structs like this by value. For example,
it irritates me that we tend to pass ItemPointers by reference, when
it's a small struct that represents a single value. I think of it as an
atomic value, like an integer, so it feels wrong to pass it by reference.

- Heikki


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Heikki Linnakangas <hlinnakangas(at)vmware(dot)com>
Cc: Tatsuo Ishii <ishii(at)postgresql(dot)org>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Question about coding of free space map
Date: 2014-08-29 14:40:02
Message-ID: 18002.1409323202@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Heikki Linnakangas <hlinnakangas(at)vmware(dot)com> writes:
> On 08/26/2014 05:13 AM, Tatsuo Ishii wrote:
>> While looking into backend/storage/freespace/freespace.c, I noticed
>> that struct FSMAddress is passed to functions by value, rather than
>> reference.

> There isn't really any strict coding rule on that. We pass RelFileNode's
> by value in many functions, for example.

The only reason RelFileNodes work like that is that Robert blithely
ignored the coding rule when he was revising things to pass those around.
I've never been terribly happy about it, but it wasn't important enough
to complain about.

The cases where it *would* be important enough to complain about would
be performance-critical code paths, which RelFileNode usages typically
don't appear in (if you're messing with one you're most likely going
to do a filesystem access). I'd be unhappy though if someone wanted
to start passing ItemPointers by value. I doubt we can rely on C
compilers to pass those as efficiently as they pass pointers.

regards, tom lane