Commit ac12ab06 authored by Tom Lane's avatar Tom Lane

Avoid trying to release a List's initial allocation via repalloc().

Commit 1cff1b95 included some code that supposed it could repalloc()
a memory chunk to a smaller size without risk of the chunk moving.
That was not a great idea, because it depended on undocumented behavior
of AllocSetRealloc, which commit c477f3e4 changed thereby breaking it.
(Not to mention that this code ought to work with other memory context
types, which might not work the same...)  So get rid of the repalloc
calls, and instead just wipe the now-unused ListCell array and/or tell
Valgrind it's NOACCESS, as if we'd freed it.

In cases where the initial list allocation had been quite large, this
could represent an annoying waste of space.  In principle we could
ameliorate that by allocating the initial cell array separately when
it exceeds some threshold.  But that would complicate new_list() which
is hot code, and the returns would materialize only in narrow cases.
On balance I don't think it'd be worth it.

Discussion: https://postgr.es/m/17059.1570208426@sss.pgh.pa.us
parent 36425ece
...@@ -18,6 +18,7 @@ ...@@ -18,6 +18,7 @@
#include "postgres.h" #include "postgres.h"
#include "nodes/pg_list.h" #include "nodes/pg_list.h"
#include "utils/memdebug.h"
#include "utils/memutils.h" #include "utils/memutils.h"
...@@ -172,8 +173,6 @@ enlarge_list(List *list, int min_size) ...@@ -172,8 +173,6 @@ enlarge_list(List *list, int min_size)
if (list->elements == list->initial_elements) if (list->elements == list->initial_elements)
{ {
List *newlist PG_USED_FOR_ASSERTS_ONLY;
/* /*
* Replace original in-line allocation with a separate palloc block. * Replace original in-line allocation with a separate palloc block.
* Ensure it is in the same memory context as the List header. (The * Ensure it is in the same memory context as the List header. (The
...@@ -188,16 +187,18 @@ enlarge_list(List *list, int min_size) ...@@ -188,16 +187,18 @@ enlarge_list(List *list, int min_size)
list->length * sizeof(ListCell)); list->length * sizeof(ListCell));
/* /*
* Currently, asking aset.c to reduce the allocated size of the List * We must not move the list header, so it's unsafe to try to reclaim
* header is pointless in terms of reclaiming space, unless the list * the initial_elements[] space via repalloc. In debugging builds,
* is very long. However, it seems worth doing anyway to cause the * however, we can clear that space and/or mark it inaccessible.
* no-longer-needed initial_elements[] space to be cleared in * (wipe_mem includes VALGRIND_MAKE_MEM_NOACCESS.)
* debugging builds. */
*/ #ifdef CLOBBER_FREED_MEMORY
newlist = (List *) repalloc(list, offsetof(List, initial_elements)); wipe_mem(list->initial_elements,
list->max_length * sizeof(ListCell));
/* That better not have failed, nor moved the list header */ #else
Assert(newlist == list); VALGRIND_MAKE_MEM_NOACCESS(list->initial_elements,
list->max_length * sizeof(ListCell));
#endif
} }
else else
{ {
...@@ -736,13 +737,16 @@ list_delete_nth_cell(List *list, int n) ...@@ -736,13 +737,16 @@ list_delete_nth_cell(List *list, int n)
else else
{ {
/* /*
* As in enlarge_list(), tell palloc code we're not using the * As in enlarge_list(), clear the initial_elements[] space and/or
* initial_elements space anymore. * mark it inaccessible.
*/ */
List *newlist PG_USED_FOR_ASSERTS_ONLY; #ifdef CLOBBER_FREED_MEMORY
wipe_mem(list->initial_elements,
newlist = (List *) repalloc(list, offsetof(List, initial_elements)); list->max_length * sizeof(ListCell));
Assert(newlist == list); #else
VALGRIND_MAKE_MEM_NOACCESS(list->initial_elements,
list->max_length * sizeof(ListCell));
#endif
} }
list->elements = newelems; list->elements = newelems;
list->max_length = newmaxlen; list->max_length = newmaxlen;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment