Re: Pre-alloc ListCell's optimization - Mailing list pgsql-hackers

From Stephen Frost
Subject Re: Pre-alloc ListCell's optimization
Date
Msg-id 20120516185044.GW1267@tamriel.snowman.net
Whole thread Raw
In response to Re: Pre-alloc ListCell's optimization  (Stephen Frost <sfrost@snowman.net>)
List pgsql-hackers
* Stephen Frost (sfrost@snowman.net) wrote:
> The two cases in clauses.c are pretty specific and documented:
>
>       List *subargs = list_copy(((BoolExpr *) arg)->args);
>
>       /* overly tense code to avoid leaking unused list header */
>       if (!unprocessed_args)
>           unprocessed_args = subargs;
>       else
>       {
>           List *oldhdr = unprocessed_args;
>
>           unprocessed_args = list_concat(subargs, unprocessed_args);
>           pfree(oldhdr);
>       }

Having thought through this a bit more, with the new list
implementation, it's possible to just replace the pfree(oldhdr); with
list_free(oldhdr); and we may want to document or perhaps encourage that
users of list_concat() consider list_free()'ing the 2nd list if memory
is a concern under the new implementation.

The list_copy() will allocate a new list into subargs.  list_concat()
with the new list implementation will just append unprocessed_args on to
the end of subargs, and with unprocessed_args being replaced and that
list only being referenced by oldhdr, we can go ahead and do a shallow
free of that list.

We may want to go back and consider other uses of list_concat() under
the new list implementation, since the amount of memory leak'd now is
larger than just the list header, it's the list header and the array of
8 initial list element slots.  Still, these were the only places where
we were worried about free'ing the list header, so perhaps the other
list_concat() uses aren't in highly called areas or are in situations
which get cleaned up by the memory context system sufficiently.

> In transformFromClauseItem(), the pfree is:
>
> pfree(r_relnamespace);  /* free unneeded list header */

Same here- this can be replaced with a list_free() in place of the
pfree() under the new list implementation.

These changes passes all regression tests, though I don't know how
heavily these areas are tested in the regression suite.
Thanks,
    Stephen

pgsql-hackers by date:

Previous
From: Teodor Sigaev
Date:
Subject: psql bug
Next
From: Jeff Janes
Date:
Subject: Re: Draft release notes complete