public inbox for [email protected]
help / color / mirror / Atom feedFrom: Nathan Bossart <[email protected]>
To: Robert Haas <[email protected]>
Cc: Tom Lane <[email protected]>
Cc: Alvaro Herrera <[email protected]>
Cc: Andrew Dunstan <[email protected]>
Cc: [email protected]
Subject: Re: Inefficiency in parallel pg_restore with many tables
Date: Sat, 2 Sep 2023 11:55:21 -0700
Message-ID: <20230902185521.GA3414119@nathanxps13> (raw)
In-Reply-To: <20230901205248.GA3184752@nathanxps13>
References: <[email protected]>
<20230718160713.GA1139177@nathanxps13>
<20230720190644.GA1724613@nathanxps13>
<20230722231941.GA2020225@nathanxps13>
<[email protected]>
<20230723055703.GA2421212@nathanxps13>
<20230724190015.GA2528613@nathanxps13>
<20230725185336.GA2911441@nathanxps13>
<CA+TgmoYtJ+ad+ej=BCPWRNPyNoLy2R2O5wBSZUeyaQ54jrPk4A@mail.gmail.com>
<20230901205248.GA3184752@nathanxps13>
On Fri, Sep 01, 2023 at 01:52:48PM -0700, Nathan Bossart wrote:
> On Fri, Sep 01, 2023 at 04:00:44PM -0400, Robert Haas wrote:
>> In hindsight, I think that making binaryheap depend on Datum was a bad
>> idea. I think that was my idea, and I think it wasn't very smart.
>> Considering that people have coded to that decision up until now, it
>> might not be too easy to change at this point. But in principle I
>> guess you'd want to be able to make a heap out of any C data type,
>> rather than just Datum, or just Datum in the backend and just void *
>> in the frontend.
>
> Yeah, something similar to simplehash for binary heaps could be nice. That
> being said, I don't know if there's a strong reason to specialize the
> implementation for a given C data type in most cases. I suspect many
> callers are just fine with dealing with pointers (e.g., I wouldn't store an
> entire TocEntry in the array), and smaller types like integers are already
> stored directly in the array thanks to the use of Datum. However, it
> _would_ allow us to abandon this frontend/backend void */Datum kludge,
> which is something.
I ended up hacking together a (nowhere near committable) patch to see how
hard it would be to allow using any type with binaryheap. It doesn't seem
too bad.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
Attachments:
[text/x-diff] 0001-allow-binaryheap-to-use-any-type.patch (22.6K, ../20230902185521.GA3414119@nathanxps13/2-0001-allow-binaryheap-to-use-any-type.patch)
download | inline diff:
From fa51a6b48d1a7cdcffbd5ccbe4274f39dfb0c1b5 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <[email protected]>
Date: Sat, 2 Sep 2023 11:48:36 -0700
Subject: [PATCH 1/1] allow binaryheap to use any type
---
src/backend/executor/nodeGatherMerge.c | 20 +++---
src/backend/executor/nodeMergeAppend.c | 20 +++---
src/backend/lib/binaryheap.c | 71 ++++++++++---------
src/backend/postmaster/pgarch.c | 37 +++++-----
.../replication/logical/reorderbuffer.c | 22 +++---
src/backend/storage/buffer/bufmgr.c | 21 +++---
src/include/lib/binaryheap.h | 17 ++---
7 files changed, 112 insertions(+), 96 deletions(-)
diff --git a/src/backend/executor/nodeGatherMerge.c b/src/backend/executor/nodeGatherMerge.c
index 9d5e1a46e9..4c75d224c8 100644
--- a/src/backend/executor/nodeGatherMerge.c
+++ b/src/backend/executor/nodeGatherMerge.c
@@ -52,7 +52,7 @@ typedef struct GMReaderTupleBuffer
} GMReaderTupleBuffer;
static TupleTableSlot *ExecGatherMerge(PlanState *pstate);
-static int32 heap_compare_slots(Datum a, Datum b, void *arg);
+static int32 heap_compare_slots(void *a, void *b, void *arg);
static TupleTableSlot *gather_merge_getnext(GatherMergeState *gm_state);
static MinimalTuple gm_readnext_tuple(GatherMergeState *gm_state, int nreader,
bool nowait, bool *done);
@@ -428,7 +428,7 @@ gather_merge_setup(GatherMergeState *gm_state)
}
/* Allocate the resources for the merge */
- gm_state->gm_heap = binaryheap_allocate(nreaders + 1,
+ gm_state->gm_heap = binaryheap_allocate(nreaders + 1, sizeof(int),
heap_compare_slots,
gm_state);
}
@@ -489,7 +489,7 @@ reread:
/* Don't have a tuple yet, try to get one */
if (gather_merge_readnext(gm_state, i, nowait))
binaryheap_add_unordered(gm_state->gm_heap,
- Int32GetDatum(i));
+ &i);
}
else
{
@@ -565,14 +565,14 @@ gather_merge_getnext(GatherMergeState *gm_state)
* the heap, because it might now compare differently against the
* other elements of the heap.
*/
- i = DatumGetInt32(binaryheap_first(gm_state->gm_heap));
+ binaryheap_first(gm_state->gm_heap, &i);
if (gather_merge_readnext(gm_state, i, false))
- binaryheap_replace_first(gm_state->gm_heap, Int32GetDatum(i));
+ binaryheap_replace_first(gm_state->gm_heap, &i);
else
{
/* reader exhausted, remove it from heap */
- (void) binaryheap_remove_first(gm_state->gm_heap);
+ binaryheap_remove_first(gm_state->gm_heap, &i);
}
}
@@ -585,7 +585,7 @@ gather_merge_getnext(GatherMergeState *gm_state)
else
{
/* Return next tuple from whichever participant has the leading one */
- i = DatumGetInt32(binaryheap_first(gm_state->gm_heap));
+ binaryheap_first(gm_state->gm_heap, &i);
return gm_state->gm_slots[i];
}
}
@@ -750,11 +750,11 @@ typedef int32 SlotNumber;
* Compare the tuples in the two given slots.
*/
static int32
-heap_compare_slots(Datum a, Datum b, void *arg)
+heap_compare_slots(void *a, void *b, void *arg)
{
GatherMergeState *node = (GatherMergeState *) arg;
- SlotNumber slot1 = DatumGetInt32(a);
- SlotNumber slot2 = DatumGetInt32(b);
+ SlotNumber slot1 = *((int *) a);
+ SlotNumber slot2 = *((int *) b);
TupleTableSlot *s1 = node->gm_slots[slot1];
TupleTableSlot *s2 = node->gm_slots[slot2];
diff --git a/src/backend/executor/nodeMergeAppend.c b/src/backend/executor/nodeMergeAppend.c
index 21b5726e6e..928b4b3719 100644
--- a/src/backend/executor/nodeMergeAppend.c
+++ b/src/backend/executor/nodeMergeAppend.c
@@ -52,7 +52,7 @@
typedef int32 SlotNumber;
static TupleTableSlot *ExecMergeAppend(PlanState *pstate);
-static int heap_compare_slots(Datum a, Datum b, void *arg);
+static int heap_compare_slots(void *a, void *b, void *arg);
/* ----------------------------------------------------------------
@@ -125,7 +125,7 @@ ExecInitMergeAppend(MergeAppend *node, EState *estate, int eflags)
mergestate->ms_nplans = nplans;
mergestate->ms_slots = (TupleTableSlot **) palloc0(sizeof(TupleTableSlot *) * nplans);
- mergestate->ms_heap = binaryheap_allocate(nplans, heap_compare_slots,
+ mergestate->ms_heap = binaryheap_allocate(nplans, sizeof(int), heap_compare_slots,
mergestate);
/*
@@ -229,7 +229,7 @@ ExecMergeAppend(PlanState *pstate)
{
node->ms_slots[i] = ExecProcNode(node->mergeplans[i]);
if (!TupIsNull(node->ms_slots[i]))
- binaryheap_add_unordered(node->ms_heap, Int32GetDatum(i));
+ binaryheap_add_unordered(node->ms_heap, &i);
}
binaryheap_build(node->ms_heap);
node->ms_initialized = true;
@@ -244,12 +244,12 @@ ExecMergeAppend(PlanState *pstate)
* by doing this before returning from the prior call, but it's better
* to not pull tuples until necessary.)
*/
- i = DatumGetInt32(binaryheap_first(node->ms_heap));
+ binaryheap_first(node->ms_heap, &i);
node->ms_slots[i] = ExecProcNode(node->mergeplans[i]);
if (!TupIsNull(node->ms_slots[i]))
- binaryheap_replace_first(node->ms_heap, Int32GetDatum(i));
+ binaryheap_replace_first(node->ms_heap, &i);
else
- (void) binaryheap_remove_first(node->ms_heap);
+ binaryheap_remove_first(node->ms_heap, &i);
}
if (binaryheap_empty(node->ms_heap))
@@ -259,7 +259,7 @@ ExecMergeAppend(PlanState *pstate)
}
else
{
- i = DatumGetInt32(binaryheap_first(node->ms_heap));
+ binaryheap_first(node->ms_heap, &i);
result = node->ms_slots[i];
}
@@ -270,11 +270,11 @@ ExecMergeAppend(PlanState *pstate)
* Compare the tuples in the two given slots.
*/
static int32
-heap_compare_slots(Datum a, Datum b, void *arg)
+heap_compare_slots(void *a, void *b, void *arg)
{
MergeAppendState *node = (MergeAppendState *) arg;
- SlotNumber slot1 = DatumGetInt32(a);
- SlotNumber slot2 = DatumGetInt32(b);
+ SlotNumber slot1 = *((int *) a);
+ SlotNumber slot2 = *((int *) b);
TupleTableSlot *s1 = node->ms_slots[slot1];
TupleTableSlot *s2 = node->ms_slots[slot2];
diff --git a/src/backend/lib/binaryheap.c b/src/backend/lib/binaryheap.c
index 1737546757..0dc0e18f48 100644
--- a/src/backend/lib/binaryheap.c
+++ b/src/backend/lib/binaryheap.c
@@ -20,6 +20,9 @@
static void sift_down(binaryheap *heap, int node_off);
static void sift_up(binaryheap *heap, int node_off);
+#define bh_node_ptr(n) (&heap->bh_nodes[(n) * heap->bh_elem_size])
+#define bh_set(n, d) (memmove(bh_node_ptr((n)), (d), heap->bh_elem_size))
+
/*
* binaryheap_allocate
*
@@ -29,14 +32,16 @@ static void sift_up(binaryheap *heap, int node_off);
* argument specified by 'arg'.
*/
binaryheap *
-binaryheap_allocate(int capacity, binaryheap_comparator compare, void *arg)
+binaryheap_allocate(int capacity, size_t elem_size,
+ binaryheap_comparator compare, void *arg)
{
int sz;
binaryheap *heap;
- sz = offsetof(binaryheap, bh_nodes) + sizeof(Datum) * capacity;
+ sz = offsetof(binaryheap, bh_nodes) + elem_size * capacity;
heap = (binaryheap *) palloc(sz);
heap->bh_space = capacity;
+ heap->bh_elem_size = elem_size;
heap->bh_compare = compare;
heap->bh_arg = arg;
@@ -106,12 +111,12 @@ parent_offset(int i)
* afterwards.
*/
void
-binaryheap_add_unordered(binaryheap *heap, Datum d)
+binaryheap_add_unordered(binaryheap *heap, void *d)
{
if (heap->bh_size >= heap->bh_space)
elog(ERROR, "out of binary heap slots");
heap->bh_has_heap_property = false;
- heap->bh_nodes[heap->bh_size] = d;
+ bh_set(heap->bh_size, d);
heap->bh_size++;
}
@@ -138,11 +143,11 @@ binaryheap_build(binaryheap *heap)
* the heap property.
*/
void
-binaryheap_add(binaryheap *heap, Datum d)
+binaryheap_add(binaryheap *heap, void *d)
{
if (heap->bh_size >= heap->bh_space)
elog(ERROR, "out of binary heap slots");
- heap->bh_nodes[heap->bh_size] = d;
+ bh_set(heap->bh_size, d);
heap->bh_size++;
sift_up(heap, heap->bh_size - 1);
}
@@ -154,11 +159,11 @@ binaryheap_add(binaryheap *heap, Datum d)
* without modifying the heap. The caller must ensure that this
* routine is not used on an empty heap. Always O(1).
*/
-Datum
-binaryheap_first(binaryheap *heap)
+void
+binaryheap_first(binaryheap *heap, void *result)
{
Assert(!binaryheap_empty(heap) && heap->bh_has_heap_property);
- return heap->bh_nodes[0];
+ memmove(result, heap->bh_nodes, heap->bh_elem_size);
}
/*
@@ -169,31 +174,27 @@ binaryheap_first(binaryheap *heap)
* that this routine is not used on an empty heap. O(log n) worst
* case.
*/
-Datum
-binaryheap_remove_first(binaryheap *heap)
+void
+binaryheap_remove_first(binaryheap *heap, void *result)
{
- Datum result;
-
Assert(!binaryheap_empty(heap) && heap->bh_has_heap_property);
/* extract the root node, which will be the result */
- result = heap->bh_nodes[0];
+ memmove(result, heap->bh_nodes, heap->bh_elem_size);
/* easy if heap contains one element */
if (heap->bh_size == 1)
{
heap->bh_size--;
- return result;
+ return;
}
/*
* Remove the last node, placing it in the vacated root entry, and sift
* the new root node down to its correct position.
*/
- heap->bh_nodes[0] = heap->bh_nodes[--heap->bh_size];
+ bh_set(0, bh_node_ptr(--heap->bh_size));
sift_down(heap, 0);
-
- return result;
}
/*
@@ -204,11 +205,11 @@ binaryheap_remove_first(binaryheap *heap)
* sifting the new node down.
*/
void
-binaryheap_replace_first(binaryheap *heap, Datum d)
+binaryheap_replace_first(binaryheap *heap, void *d)
{
Assert(!binaryheap_empty(heap) && heap->bh_has_heap_property);
- heap->bh_nodes[0] = d;
+ bh_set(0, d);
if (heap->bh_size > 1)
sift_down(heap, 0);
@@ -221,7 +222,9 @@ binaryheap_replace_first(binaryheap *heap, Datum d)
static void
sift_up(binaryheap *heap, int node_off)
{
- Datum node_val = heap->bh_nodes[node_off];
+ char *node_val = palloc(heap->bh_elem_size);
+
+ memmove(node_val, bh_node_ptr(node_off), heap->bh_elem_size);
/*
* Within the loop, the node_off'th array entry is a "hole" that
@@ -232,14 +235,14 @@ sift_up(binaryheap *heap, int node_off)
{
int cmp;
int parent_off;
- Datum parent_val;
+ void *parent_val;
/*
* If this node is smaller than its parent, the heap condition is
* satisfied, and we're done.
*/
parent_off = parent_offset(node_off);
- parent_val = heap->bh_nodes[parent_off];
+ parent_val = bh_node_ptr(parent_off);
cmp = heap->bh_compare(node_val,
parent_val,
heap->bh_arg);
@@ -250,11 +253,12 @@ sift_up(binaryheap *heap, int node_off)
* Otherwise, swap the parent value with the hole, and go on to check
* the node's new parent.
*/
- heap->bh_nodes[node_off] = parent_val;
+ bh_set(node_off, parent_val);
node_off = parent_off;
}
/* Re-fill the hole */
- heap->bh_nodes[node_off] = node_val;
+ bh_set(node_off, node_val);
+ pfree(node_val);
}
/*
@@ -264,7 +268,9 @@ sift_up(binaryheap *heap, int node_off)
static void
sift_down(binaryheap *heap, int node_off)
{
- Datum node_val = heap->bh_nodes[node_off];
+ char *node_val = palloc(heap->bh_elem_size);
+
+ memmove(node_val, bh_node_ptr(node_off), heap->bh_elem_size);
/*
* Within the loop, the node_off'th array entry is a "hole" that
@@ -280,20 +286,20 @@ sift_down(binaryheap *heap, int node_off)
/* Is the left child larger than the parent? */
if (left_off < heap->bh_size &&
heap->bh_compare(node_val,
- heap->bh_nodes[left_off],
+ bh_node_ptr(left_off),
heap->bh_arg) < 0)
swap_off = left_off;
/* Is the right child larger than the parent? */
if (right_off < heap->bh_size &&
heap->bh_compare(node_val,
- heap->bh_nodes[right_off],
+ bh_node_ptr(right_off),
heap->bh_arg) < 0)
{
/* swap with the larger child */
if (!swap_off ||
- heap->bh_compare(heap->bh_nodes[left_off],
- heap->bh_nodes[right_off],
+ heap->bh_compare(bh_node_ptr(left_off),
+ bh_node_ptr(right_off),
heap->bh_arg) < 0)
swap_off = right_off;
}
@@ -309,9 +315,10 @@ sift_down(binaryheap *heap, int node_off)
* Otherwise, swap the hole with the child that violates the heap
* property; then go on to check its children.
*/
- heap->bh_nodes[node_off] = heap->bh_nodes[swap_off];
+ bh_set(node_off, bh_node_ptr(swap_off));
node_off = swap_off;
}
/* Re-fill the hole */
- heap->bh_nodes[node_off] = node_val;
+ bh_set(node_off, node_val);
+ pfree(node_val);
}
diff --git a/src/backend/postmaster/pgarch.c b/src/backend/postmaster/pgarch.c
index 46af349564..a11005708a 100644
--- a/src/backend/postmaster/pgarch.c
+++ b/src/backend/postmaster/pgarch.c
@@ -145,7 +145,7 @@ static bool pgarch_readyXlog(char *xlog);
static void pgarch_archiveDone(char *xlog);
static void pgarch_die(int code, Datum arg);
static void HandlePgArchInterrupts(void);
-static int ready_file_comparator(Datum a, Datum b, void *arg);
+static int ready_file_comparator(void *a, void *b, void *arg);
static void LoadArchiveLibrary(void);
static void pgarch_call_module_shutdown_cb(int code, Datum arg);
@@ -249,7 +249,7 @@ PgArchiverMain(void)
arch_files->arch_files_size = 0;
/* Initialize our max-heap for prioritizing files to archive. */
- arch_files->arch_heap = binaryheap_allocate(NUM_FILES_PER_DIRECTORY_SCAN,
+ arch_files->arch_heap = binaryheap_allocate(NUM_FILES_PER_DIRECTORY_SCAN, sizeof(char *),
ready_file_comparator, NULL);
/* Load the archive_library. */
@@ -631,22 +631,27 @@ pgarch_readyXlog(char *xlog)
/* If the heap isn't full yet, quickly add it. */
arch_file = arch_files->arch_filenames[arch_files->arch_heap->bh_size];
strcpy(arch_file, basename);
- binaryheap_add_unordered(arch_files->arch_heap, CStringGetDatum(arch_file));
+ binaryheap_add_unordered(arch_files->arch_heap, &arch_file);
/* If we just filled the heap, make it a valid one. */
if (arch_files->arch_heap->bh_size == NUM_FILES_PER_DIRECTORY_SCAN)
binaryheap_build(arch_files->arch_heap);
}
- else if (ready_file_comparator(binaryheap_first(arch_files->arch_heap),
- CStringGetDatum(basename), NULL) > 0)
+ else
{
- /*
- * Remove the lowest priority file and add the current one to the
- * heap.
- */
- arch_file = DatumGetCString(binaryheap_remove_first(arch_files->arch_heap));
- strcpy(arch_file, basename);
- binaryheap_add(arch_files->arch_heap, CStringGetDatum(arch_file));
+ char *result;
+
+ binaryheap_first(arch_files->arch_heap, &result);
+ if (ready_file_comparator(&result, &basename, NULL) > 0)
+ {
+ /*
+ * Remove the lowest priority file and add the current one to
+ * the heap.
+ */
+ binaryheap_remove_first(arch_files->arch_heap, &arch_file);
+ strcpy(arch_file, basename);
+ binaryheap_add(arch_files->arch_heap, &arch_file);
+ }
}
}
FreeDir(rldir);
@@ -668,7 +673,7 @@ pgarch_readyXlog(char *xlog)
*/
arch_files->arch_files_size = arch_files->arch_heap->bh_size;
for (int i = 0; i < arch_files->arch_files_size; i++)
- arch_files->arch_files[i] = DatumGetCString(binaryheap_remove_first(arch_files->arch_heap));
+ binaryheap_remove_first(arch_files->arch_heap, &arch_files->arch_files[i]);
/* Return the highest priority file. */
arch_files->arch_files_size--;
@@ -686,10 +691,10 @@ pgarch_readyXlog(char *xlog)
* If "a" and "b" have equivalent values, 0 will be returned.
*/
static int
-ready_file_comparator(Datum a, Datum b, void *arg)
+ready_file_comparator(void *a, void *b, void *arg)
{
- char *a_str = DatumGetCString(a);
- char *b_str = DatumGetCString(b);
+ char *a_str = *((char **) a);
+ char *b_str = *((char **) b);
bool a_history = IsTLHistoryFileName(a_str);
bool b_history = IsTLHistoryFileName(b_str);
diff --git a/src/backend/replication/logical/reorderbuffer.c b/src/backend/replication/logical/reorderbuffer.c
index 12edc5772a..b0a84c5ed1 100644
--- a/src/backend/replication/logical/reorderbuffer.c
+++ b/src/backend/replication/logical/reorderbuffer.c
@@ -1223,11 +1223,11 @@ ReorderBufferCommitChild(ReorderBuffer *rb, TransactionId xid,
* Binary heap comparison function.
*/
static int
-ReorderBufferIterCompare(Datum a, Datum b, void *arg)
+ReorderBufferIterCompare(void *a, void *b, void *arg)
{
ReorderBufferIterTXNState *state = (ReorderBufferIterTXNState *) arg;
- XLogRecPtr pos_a = state->entries[DatumGetInt32(a)].lsn;
- XLogRecPtr pos_b = state->entries[DatumGetInt32(b)].lsn;
+ XLogRecPtr pos_a = state->entries[*((int *) a)].lsn;
+ XLogRecPtr pos_b = state->entries[*((int *) b)].lsn;
if (pos_a < pos_b)
return 1;
@@ -1296,7 +1296,7 @@ ReorderBufferIterTXNInit(ReorderBuffer *rb, ReorderBufferTXN *txn,
}
/* allocate heap */
- state->heap = binaryheap_allocate(state->nr_txns,
+ state->heap = binaryheap_allocate(state->nr_txns, sizeof(int),
ReorderBufferIterCompare,
state);
@@ -1330,7 +1330,8 @@ ReorderBufferIterTXNInit(ReorderBuffer *rb, ReorderBufferTXN *txn,
state->entries[off].change = cur_change;
state->entries[off].txn = txn;
- binaryheap_add_unordered(state->heap, Int32GetDatum(off++));
+ binaryheap_add_unordered(state->heap, &off);
+ off++;
}
/* add subtransactions if they contain changes */
@@ -1359,7 +1360,8 @@ ReorderBufferIterTXNInit(ReorderBuffer *rb, ReorderBufferTXN *txn,
state->entries[off].change = cur_change;
state->entries[off].txn = cur_txn;
- binaryheap_add_unordered(state->heap, Int32GetDatum(off++));
+ binaryheap_add_unordered(state->heap, &off);
+ off++;
}
}
@@ -1384,7 +1386,7 @@ ReorderBufferIterTXNNext(ReorderBuffer *rb, ReorderBufferIterTXNState *state)
if (state->heap->bh_size == 0)
return NULL;
- off = DatumGetInt32(binaryheap_first(state->heap));
+ binaryheap_first(state->heap, &off);
entry = &state->entries[off];
/* free memory we might have "leaked" in the previous *Next call */
@@ -1414,7 +1416,7 @@ ReorderBufferIterTXNNext(ReorderBuffer *rb, ReorderBufferIterTXNState *state)
state->entries[off].lsn = next_change->lsn;
state->entries[off].change = next_change;
- binaryheap_replace_first(state->heap, Int32GetDatum(off));
+ binaryheap_replace_first(state->heap, &off);
return change;
}
@@ -1450,14 +1452,14 @@ ReorderBufferIterTXNNext(ReorderBuffer *rb, ReorderBufferIterTXNState *state)
/* txn stays the same */
state->entries[off].lsn = next_change->lsn;
state->entries[off].change = next_change;
- binaryheap_replace_first(state->heap, Int32GetDatum(off));
+ binaryheap_replace_first(state->heap, &off);
return change;
}
}
/* ok, no changes there anymore, remove */
- binaryheap_remove_first(state->heap);
+ binaryheap_remove_first(state->heap, &off);
return change;
}
diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c
index 3bd82dbfca..5bd7176b3c 100644
--- a/src/backend/storage/buffer/bufmgr.c
+++ b/src/backend/storage/buffer/bufmgr.c
@@ -501,7 +501,7 @@ static void CheckForBufferLeaks(void);
static int rlocator_comparator(const void *p1, const void *p2);
static inline int buffertag_comparator(const BufferTag *ba, const BufferTag *bb);
static inline int ckpt_buforder_comparator(const CkptSortItem *a, const CkptSortItem *b);
-static int ts_ckpt_progress_comparator(Datum a, Datum b, void *arg);
+static int ts_ckpt_progress_comparator(void *a, void *b, void *arg);
/*
@@ -2639,7 +2639,7 @@ BufferSync(int flags)
* and compute how large a portion of the total progress a single
* processed buffer is.
*/
- ts_heap = binaryheap_allocate(num_spaces,
+ ts_heap = binaryheap_allocate(num_spaces, sizeof(CkptTsStatus *),
ts_ckpt_progress_comparator,
NULL);
@@ -2649,7 +2649,7 @@ BufferSync(int flags)
ts_stat->progress_slice = (float8) num_to_scan / ts_stat->num_to_scan;
- binaryheap_add_unordered(ts_heap, PointerGetDatum(ts_stat));
+ binaryheap_add_unordered(ts_heap, &ts_stat);
}
binaryheap_build(ts_heap);
@@ -2665,8 +2665,9 @@ BufferSync(int flags)
while (!binaryheap_empty(ts_heap))
{
BufferDesc *bufHdr = NULL;
- CkptTsStatus *ts_stat = (CkptTsStatus *)
- DatumGetPointer(binaryheap_first(ts_heap));
+ CkptTsStatus *ts_stat;
+
+ binaryheap_first(ts_heap, &ts_stat);
buf_id = CkptBufferIds[ts_stat->index].buf_id;
Assert(buf_id != -1);
@@ -2708,12 +2709,12 @@ BufferSync(int flags)
/* Have all the buffers from the tablespace been processed? */
if (ts_stat->num_scanned == ts_stat->num_to_scan)
{
- binaryheap_remove_first(ts_heap);
+ binaryheap_remove_first(ts_heap, &ts_stat);
}
else
{
/* update heap with the new progress */
- binaryheap_replace_first(ts_heap, PointerGetDatum(ts_stat));
+ binaryheap_replace_first(ts_heap, &ts_stat);
}
/*
@@ -5416,10 +5417,10 @@ ckpt_buforder_comparator(const CkptSortItem *a, const CkptSortItem *b)
* progress.
*/
static int
-ts_ckpt_progress_comparator(Datum a, Datum b, void *arg)
+ts_ckpt_progress_comparator(void *a, void *b, void *arg)
{
- CkptTsStatus *sa = (CkptTsStatus *) a;
- CkptTsStatus *sb = (CkptTsStatus *) b;
+ CkptTsStatus *sa = *((CkptTsStatus **) a);
+ CkptTsStatus *sb = *((CkptTsStatus **) b);
/* we want a min-heap, so return 1 for the a < b */
if (sa->progress < sb->progress)
diff --git a/src/include/lib/binaryheap.h b/src/include/lib/binaryheap.h
index 52f7b06b25..24228e5b4b 100644
--- a/src/include/lib/binaryheap.h
+++ b/src/include/lib/binaryheap.h
@@ -15,7 +15,7 @@
* For a max-heap, the comparator must return <0 iff a < b, 0 iff a == b,
* and >0 iff a > b. For a min-heap, the conditions are reversed.
*/
-typedef int (*binaryheap_comparator) (Datum a, Datum b, void *arg);
+typedef int (*binaryheap_comparator) (void *a, void *b, void *arg);
/*
* binaryheap
@@ -31,23 +31,24 @@ typedef struct binaryheap
{
int bh_size;
int bh_space;
+ size_t bh_elem_size;
bool bh_has_heap_property; /* debugging cross-check */
binaryheap_comparator bh_compare;
void *bh_arg;
- Datum bh_nodes[FLEXIBLE_ARRAY_MEMBER];
+ char bh_nodes[FLEXIBLE_ARRAY_MEMBER];
} binaryheap;
-extern binaryheap *binaryheap_allocate(int capacity,
+extern binaryheap *binaryheap_allocate(int capacity, size_t elem_size,
binaryheap_comparator compare,
void *arg);
extern void binaryheap_reset(binaryheap *heap);
extern void binaryheap_free(binaryheap *heap);
-extern void binaryheap_add_unordered(binaryheap *heap, Datum d);
+extern void binaryheap_add_unordered(binaryheap *heap, void *d);
extern void binaryheap_build(binaryheap *heap);
-extern void binaryheap_add(binaryheap *heap, Datum d);
-extern Datum binaryheap_first(binaryheap *heap);
-extern Datum binaryheap_remove_first(binaryheap *heap);
-extern void binaryheap_replace_first(binaryheap *heap, Datum d);
+extern void binaryheap_add(binaryheap *heap, void *d);
+extern void binaryheap_first(binaryheap *heap, void *result);
+extern void binaryheap_remove_first(binaryheap *heap, void *result);
+extern void binaryheap_replace_first(binaryheap *heap, void *d);
#define binaryheap_empty(h) ((h)->bh_size == 0)
--
2.25.1
view thread (23+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: Inefficiency in parallel pg_restore with many tables
In-Reply-To: <20230902185521.GA3414119@nathanxps13>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox