Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit 1cff1b9

Browse files
committedJul 15, 2019
Represent Lists as expansible arrays, not chains of cons-cells.
Originally, Postgres Lists were a more or less exact reimplementation of Lisp lists, which consist of chains of separately-allocated cons cells, each having a value and a next-cell link. We'd hacked that once before (commit d0b4399) to add a separate List header, but the data was still in cons cells. That makes some operations -- notably list_nth() -- O(N), and it's bulky because of the next-cell pointers and per-cell palloc overhead, and it's very cache-unfriendly if the cons cells end up scattered around rather than being adjacent. In this rewrite, we still have List headers, but the data is in a resizable array of values, with no next-cell links. Now we need at most two palloc's per List, and often only one, since we can allocate some values in the same palloc call as the List header. (Of course, extending an existing List may require repalloc's to enlarge the array. But this involves just O(log N) allocations not O(N).) Of course this is not without downsides. The key difficulty is that addition or deletion of a list entry may now cause other entries to move, which it did not before. For example, that breaks foreach() and sister macros, which historically used a pointer to the current cons-cell as loop state. We can repair those macros transparently by making their actual loop state be an integer list index; the exposed "ListCell *" pointer is no longer state carried across loop iterations, but is just a derived value. (In practice, modern compilers can optimize things back to having just one loop state value, at least for simple cases with inline loop bodies.) In principle, this is a semantics change for cases where the loop body inserts or deletes list entries ahead of the current loop index; but I found no such cases in the Postgres code. The change is not at all transparent for code that doesn't use foreach() but chases lists "by hand" using lnext(). The largest share of such code in the backend is in loops that were maintaining "prev" and "next" variables in addition to the current-cell pointer, in order to delete list cells efficiently using list_delete_cell(). However, we no longer need a previous-cell pointer to delete a list cell efficiently. Keeping a next-cell pointer doesn't work, as explained above, but we can improve matters by changing such code to use a regular foreach() loop and then using the new macro foreach_delete_current() to delete the current cell. (This macro knows how to update the associated foreach loop's state so that no cells will be missed in the traversal.) There remains a nontrivial risk of code assuming that a ListCell * pointer will remain good over an operation that could now move the list contents. To help catch such errors, list.c can be compiled with a new define symbol DEBUG_LIST_MEMORY_USAGE that forcibly moves list contents whenever that could possibly happen. This makes list operations significantly more expensive so it's not normally turned on (though it is on by default if USE_VALGRIND is on). There are two notable API differences from the previous code: * lnext() now requires the List's header pointer in addition to the current cell's address. * list_delete_cell() no longer requires a previous-cell argument. These changes are somewhat unfortunate, but on the other hand code using either function needs inspection to see if it is assuming anything it shouldn't, so it's not all bad. Programmers should be aware of these significant performance changes: * list_nth() and related functions are now O(1); so there's no major access-speed difference between a list and an array. * Inserting or deleting a list element now takes time proportional to the distance to the end of the list, due to moving the array elements. (However, it typically *doesn't* require palloc or pfree, so except in long lists it's probably still faster than before.) Notably, lcons() used to be about the same cost as lappend(), but that's no longer true if the list is long. Code that uses lcons() and list_delete_first() to maintain a stack might usefully be rewritten to push and pop at the end of the list rather than the beginning. * There are now list_insert_nth...() and list_delete_nth...() functions that add or remove a list cell identified by index. These have the data-movement penalty explained above, but there's no search penalty. * list_concat() and variants now copy the second list's data into storage belonging to the first list, so there is no longer any sharing of cells between the input lists. The second argument is now declared "const List *" to reflect that it isn't changed. This patch just does the minimum needed to get the new implementation in place and fix bugs exposed by the regression tests. As suggested by the foregoing, there's a fair amount of followup work remaining to do. Also, the ENABLE_LIST_COMPAT macros are finally removed in this commit. Code using those should have been gone a dozen years ago. Patch by me; thanks to David Rowley, Jesper Pedersen, and others for review. Discussion: https://postgr.es/m/[email protected]
1 parent 67b9b3c commit 1cff1b9

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

86 files changed

+1248
-1115
lines changed
 

‎contrib/file_fdw/file_fdw.c

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -360,8 +360,7 @@ fileGetOptions(Oid foreigntableid,
360360
ForeignServer *server;
361361
ForeignDataWrapper *wrapper;
362362
List *options;
363-
ListCell *lc,
364-
*prev;
363+
ListCell *lc;
365364

366365
/*
367366
* Extract options from FDW objects. We ignore user mappings because
@@ -387,25 +386,23 @@ fileGetOptions(Oid foreigntableid,
387386
*/
388387
*filename = NULL;
389388
*is_program = false;
390-
prev = NULL;
391389
foreach(lc, options)
392390
{
393391
DefElem *def = (DefElem *) lfirst(lc);
394392

395393
if (strcmp(def->defname, "filename") == 0)
396394
{
397395
*filename = defGetString(def);
398-
options = list_delete_cell(options, lc, prev);
396+
options = foreach_delete_current(options, lc);
399397
break;
400398
}
401399
else if (strcmp(def->defname, "program") == 0)
402400
{
403401
*filename = defGetString(def);
404402
*is_program = true;
405-
options = list_delete_cell(options, lc, prev);
403+
options = foreach_delete_current(options, lc);
406404
break;
407405
}
408-
prev = lc;
409406
}
410407

411408
/*

‎contrib/pg_trgm/trgm_regexp.c

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1013,9 +1013,7 @@ addKey(TrgmNFA *trgmNFA, TrgmState *state, TrgmStateKey *key)
10131013
{
10141014
regex_arc_t *arcs;
10151015
TrgmStateKey destKey;
1016-
ListCell *cell,
1017-
*prev,
1018-
*next;
1016+
ListCell *cell;
10191017
int i,
10201018
arcsCount;
10211019

@@ -1030,13 +1028,10 @@ addKey(TrgmNFA *trgmNFA, TrgmState *state, TrgmStateKey *key)
10301028
* redundancy. We can drop either old key(s) or the new key if we find
10311029
* redundancy.
10321030
*/
1033-
prev = NULL;
1034-
cell = list_head(state->enterKeys);
1035-
while (cell)
1031+
foreach(cell, state->enterKeys)
10361032
{
10371033
TrgmStateKey *existingKey = (TrgmStateKey *) lfirst(cell);
10381034

1039-
next = lnext(cell);
10401035
if (existingKey->nstate == key->nstate)
10411036
{
10421037
if (prefixContains(&existingKey->prefix, &key->prefix))
@@ -1050,15 +1045,10 @@ addKey(TrgmNFA *trgmNFA, TrgmState *state, TrgmStateKey *key)
10501045
* The new key covers this old key. Remove the old key, it's
10511046
* no longer needed once we add this key to the list.
10521047
*/
1053-
state->enterKeys = list_delete_cell(state->enterKeys,
1054-
cell, prev);
1048+
state->enterKeys = foreach_delete_current(state->enterKeys,
1049+
cell);
10551050
}
1056-
else
1057-
prev = cell;
10581051
}
1059-
else
1060-
prev = cell;
1061-
cell = next;
10621052
}
10631053

10641054
/* No redundancy, so add this key to the state's list */

0 commit comments

Comments
 (0)