Re: 0/NULL/NIL assignments in build_*_rel()

On Mon, Feb 6, 2017 at 12:04 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Ashutosh Bapat <ashutosh(dot)bapat(at)enterprisedb(dot)com> writes:
>> Both build_simple_rel() and build_join_rel() allocate RelOptInfo using
>> makeNode(), which returned a zeroed out memory. The functions then
>> assign values like false, NULL, 0 or NIL which essentially contain
>> zero valued bytes. This looks like needless work. So, we are spending
>> some CPU cycles unnecessarily in those assignments. That may not be
>> much time wasted, but whenever someone adds a field to RelOptInfo,
>> those functions need to be updated with possibly a zero value
>> assignment. That looks like an unnecessary maintenance burden. Should
>> we just drop all those zero value assignments from there?
>
> I'd vote for not. The general programming style in the backend is to
> ignore the fact that makeNode() zeroes the node's storage and initialize
> all the fields explicitly.

I know that's your preference so I try not to spend too much time
arguing about it but personally I don't like it. If I want to find
the places where a particular structure member gets set to a value,
the places where it's getting set to NULL aren't interesting, because
I have to think about that case anyway; somebody might have inserted a
makeNode() call without explicit initializations someplace; people do
that sometimes. So for me, the places where we reinitialize storage
that is already-zeroed seems like a waste not only of cycles but of
brainpower. However, as I say, I recognize that we see the world
differently on this point.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company