1
0
mirror of https://github.com/postgres/postgres.git synced 2025-05-03 22:24:49 +03:00

Improve sift up/down code in binaryheap.c and logtape.c.

Borrow the logic that's long been used in tuplesort.c: instead
of physically swapping the data in two heap entries, keep the
value that's being sifted up or down in a local variable, and
just move the other values as necessary.  This makes the code
shorter as well as faster.  It's not clear that any current
callers are really time-critical enough to notice, but we
might as well code heap maintenance the same way everywhere.

Ma Liangzhu and Tom Lane

Discussion: https://postgr.es/m/17336-fc4e522d26a750fd@postgresql.org
This commit is contained in:
Tom Lane 2021-12-14 13:35:22 -05:00
parent 2de3c1015c
commit a2ff18e89f
2 changed files with 63 additions and 61 deletions

View File

@ -19,7 +19,6 @@
static void sift_down(binaryheap *heap, int node_off); static void sift_down(binaryheap *heap, int node_off);
static void sift_up(binaryheap *heap, int node_off); static void sift_up(binaryheap *heap, int node_off);
static inline void swap_nodes(binaryheap *heap, int a, int b);
/* /*
* binaryheap_allocate * binaryheap_allocate
@ -173,24 +172,28 @@ binaryheap_first(binaryheap *heap)
Datum Datum
binaryheap_remove_first(binaryheap *heap) binaryheap_remove_first(binaryheap *heap)
{ {
Datum result;
Assert(!binaryheap_empty(heap) && heap->bh_has_heap_property); Assert(!binaryheap_empty(heap) && heap->bh_has_heap_property);
/* extract the root node, which will be the result */
result = heap->bh_nodes[0];
/* easy if heap contains one element */
if (heap->bh_size == 1) if (heap->bh_size == 1)
{ {
heap->bh_size--; heap->bh_size--;
return heap->bh_nodes[0]; return result;
} }
/* /*
* Swap the root and last nodes, decrease the size of the heap (i.e. * Remove the last node, placing it in the vacated root entry, and sift
* remove the former root node) and sift the new root node down to its * the new root node down to its correct position.
* correct position.
*/ */
swap_nodes(heap, 0, heap->bh_size - 1); heap->bh_nodes[0] = heap->bh_nodes[--heap->bh_size];
heap->bh_size--;
sift_down(heap, 0); sift_down(heap, 0);
return heap->bh_nodes[heap->bh_size]; return result;
} }
/* /*
@ -211,19 +214,6 @@ binaryheap_replace_first(binaryheap *heap, Datum d)
sift_down(heap, 0); sift_down(heap, 0);
} }
/*
* Swap the contents of two nodes.
*/
static inline void
swap_nodes(binaryheap *heap, int a, int b)
{
Datum swap;
swap = heap->bh_nodes[a];
heap->bh_nodes[a] = heap->bh_nodes[b];
heap->bh_nodes[b] = swap;
}
/* /*
* Sift a node up to the highest position it can hold according to the * Sift a node up to the highest position it can hold according to the
* comparator. * comparator.
@ -231,29 +221,40 @@ swap_nodes(binaryheap *heap, int a, int b)
static void static void
sift_up(binaryheap *heap, int node_off) sift_up(binaryheap *heap, int node_off)
{ {
Datum node_val = heap->bh_nodes[node_off];
/*
* Within the loop, the node_off'th array entry is a "hole" that
* notionally holds node_val, but we don't actually store node_val there
* till the end, saving some unnecessary data copying steps.
*/
while (node_off != 0) while (node_off != 0)
{ {
int cmp; int cmp;
int parent_off; int parent_off;
Datum parent_val;
/* /*
* If this node is smaller than its parent, the heap condition is * If this node is smaller than its parent, the heap condition is
* satisfied, and we're done. * satisfied, and we're done.
*/ */
parent_off = parent_offset(node_off); parent_off = parent_offset(node_off);
cmp = heap->bh_compare(heap->bh_nodes[node_off], parent_val = heap->bh_nodes[parent_off];
heap->bh_nodes[parent_off], cmp = heap->bh_compare(node_val,
parent_val,
heap->bh_arg); heap->bh_arg);
if (cmp <= 0) if (cmp <= 0)
break; break;
/* /*
* Otherwise, swap the node and its parent and go on to check the * Otherwise, swap the parent value with the hole, and go on to check
* node's new parent. * the node's new parent.
*/ */
swap_nodes(heap, node_off, parent_off); heap->bh_nodes[node_off] = parent_val;
node_off = parent_off; node_off = parent_off;
} }
/* Re-fill the hole */
heap->bh_nodes[node_off] = node_val;
} }
/* /*
@ -263,6 +264,13 @@ sift_up(binaryheap *heap, int node_off)
static void static void
sift_down(binaryheap *heap, int node_off) sift_down(binaryheap *heap, int node_off)
{ {
Datum node_val = heap->bh_nodes[node_off];
/*
* Within the loop, the node_off'th array entry is a "hole" that
* notionally holds node_val, but we don't actually store node_val there
* till the end, saving some unnecessary data copying steps.
*/
while (true) while (true)
{ {
int left_off = left_offset(node_off); int left_off = left_offset(node_off);
@ -271,14 +279,14 @@ sift_down(binaryheap *heap, int node_off)
/* Is the left child larger than the parent? */ /* Is the left child larger than the parent? */
if (left_off < heap->bh_size && if (left_off < heap->bh_size &&
heap->bh_compare(heap->bh_nodes[node_off], heap->bh_compare(node_val,
heap->bh_nodes[left_off], heap->bh_nodes[left_off],
heap->bh_arg) < 0) heap->bh_arg) < 0)
swap_off = left_off; swap_off = left_off;
/* Is the right child larger than the parent? */ /* Is the right child larger than the parent? */
if (right_off < heap->bh_size && if (right_off < heap->bh_size &&
heap->bh_compare(heap->bh_nodes[node_off], heap->bh_compare(node_val,
heap->bh_nodes[right_off], heap->bh_nodes[right_off],
heap->bh_arg) < 0) heap->bh_arg) < 0)
{ {
@ -298,10 +306,12 @@ sift_down(binaryheap *heap, int node_off)
break; break;
/* /*
* Otherwise, swap the node with the child that violates the heap * Otherwise, swap the hole with the child that violates the heap
* property; then go on to check its children. * property; then go on to check its children.
*/ */
swap_nodes(heap, swap_off, node_off); heap->bh_nodes[node_off] = heap->bh_nodes[swap_off];
node_off = swap_off; node_off = swap_off;
} }
/* Re-fill the hole */
heap->bh_nodes[node_off] = node_val;
} }

View File

@ -340,16 +340,6 @@ ltsReadFillBuffer(LogicalTape *lt)
return (lt->nbytes > 0); return (lt->nbytes > 0);
} }
static inline void
swap_nodes(long *heap, unsigned long a, unsigned long b)
{
long swap;
swap = heap[a];
heap[a] = heap[b];
heap[b] = swap;
}
static inline unsigned long static inline unsigned long
left_offset(unsigned long i) left_offset(unsigned long i)
{ {
@ -390,31 +380,33 @@ ltsGetFreeBlock(LogicalTapeSet *lts)
long *heap = lts->freeBlocks; long *heap = lts->freeBlocks;
long blocknum; long blocknum;
int heapsize; int heapsize;
unsigned long pos; long holeval;
unsigned long holepos;
/* freelist empty; allocate a new block */ /* freelist empty; allocate a new block */
if (lts->nFreeBlocks == 0) if (lts->nFreeBlocks == 0)
return lts->nBlocksAllocated++; return lts->nBlocksAllocated++;
/* easy if heap contains one element */
if (lts->nFreeBlocks == 1) if (lts->nFreeBlocks == 1)
{ {
lts->nFreeBlocks--; lts->nFreeBlocks--;
return lts->freeBlocks[0]; return lts->freeBlocks[0];
} }
/* take top of minheap */ /* remove top of minheap */
blocknum = heap[0]; blocknum = heap[0];
/* replace with end of minheap array */ /* we'll replace it with end of minheap array */
heap[0] = heap[--lts->nFreeBlocks]; holeval = heap[--lts->nFreeBlocks];
/* sift down */ /* sift down */
pos = 0; holepos = 0; /* holepos is where the "hole" is */
heapsize = lts->nFreeBlocks; heapsize = lts->nFreeBlocks;
while (true) while (true)
{ {
unsigned long left = left_offset(pos); unsigned long left = left_offset(holepos);
unsigned long right = right_offset(pos); unsigned long right = right_offset(holepos);
unsigned long min_child; unsigned long min_child;
if (left < heapsize && right < heapsize) if (left < heapsize && right < heapsize)
@ -426,12 +418,13 @@ ltsGetFreeBlock(LogicalTapeSet *lts)
else else
break; break;
if (heap[min_child] >= heap[pos]) if (heap[min_child] >= holeval)
break; break;
swap_nodes(heap, min_child, pos); heap[holepos] = heap[min_child];
pos = min_child; holepos = min_child;
} }
heap[holepos] = holeval;
return blocknum; return blocknum;
} }
@ -483,7 +476,7 @@ static void
ltsReleaseBlock(LogicalTapeSet *lts, long blocknum) ltsReleaseBlock(LogicalTapeSet *lts, long blocknum)
{ {
long *heap; long *heap;
unsigned long pos; unsigned long holepos;
/* /*
* Do nothing if we're no longer interested in remembering free space. * Do nothing if we're no longer interested in remembering free space.
@ -508,24 +501,23 @@ ltsReleaseBlock(LogicalTapeSet *lts, long blocknum)
lts->freeBlocksLen * sizeof(long)); lts->freeBlocksLen * sizeof(long));
} }
/* create a "hole" at end of minheap array */
heap = lts->freeBlocks; heap = lts->freeBlocks;
pos = lts->nFreeBlocks; holepos = lts->nFreeBlocks;
/* place entry at end of minheap array */
heap[pos] = blocknum;
lts->nFreeBlocks++; lts->nFreeBlocks++;
/* sift up */ /* sift up to insert blocknum */
while (pos != 0) while (holepos != 0)
{ {
unsigned long parent = parent_offset(pos); unsigned long parent = parent_offset(holepos);
if (heap[parent] < heap[pos]) if (heap[parent] < blocknum)
break; break;
swap_nodes(heap, parent, pos); heap[holepos] = heap[parent];
pos = parent; holepos = parent;
} }
heap[holepos] = blocknum;
} }
/* /*