On Mon, Feb 6, 2017 at 10:34 AM, Tom Lane <t...@sss.pgh.pa.us> wrote: > Ashutosh Bapat <ashutosh.ba...@enterprisedb.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. The primary advantage of that, IMO, is that > you can grep for references to a particular field and understand > everyplace that affects it. As an example of the value, if you want to > add a field X and you try to figure out what you need to modify by > grepping for references to existing field Y, with this proposal you would > miss places that were node initializations unless Y *always* needed to be > initialized nonzero.
In the case of adding a new field, I would rather rely on where the structure itself is used rather than another member. For example while adding a field to RelOptInfo, to find out places to initialize it, I would grep for makeNode(RelOptInfo) or such to find where a new node gets allocated, rather than say relids. Grepping for usages of a particular field might find zero valued assignments useful, but not necessarily worth maintaining that code. > > I could be convinced that it is a good idea to depart from this theory > in places where it makes a significant performance difference ... but > you've not offered any reason to think relnode.c is one. > I don't think that will bring any measurable performance improvement, unless we start creating millions of RelOptInfos in a query. My real pain is >> 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 at least move those assignments into a separate function? -- Best Wishes, Ashutosh Bapat EnterpriseDB Corporation The Postgres Database Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers