From: | Stephen Frost <sfrost(at)snowman(dot)net> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: Pre-alloc ListCell's optimization |
Date: | 2012-05-16 18:50:44 |
Message-ID: | 20120516185044.GW1267@tamriel.snowman.net |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
* Stephen Frost (sfrost(at)snowman(dot)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
From | Date | Subject | |
---|---|---|---|
Next Message | Jeff Janes | 2012-05-16 19:38:15 | Re: Draft release notes complete |
Previous Message | Teodor Sigaev | 2012-05-16 18:35:48 | psql bug |