From b570460b19c5955c91b1fc28e8fdc791f41998e2 Mon Sep 17 00:00:00 2001 From: Nathan Bossart Date: Sat, 22 Jul 2023 20:59:46 -0700 Subject: [PATCH v4 1/4] use "void *" instead of "Datum" in binaryheap --- src/backend/executor/nodeGatherMerge.c | 16 +++++----- src/backend/executor/nodeMergeAppend.c | 16 +++++----- src/backend/lib/binaryheap.c | 20 ++++++------ src/backend/postmaster/pgarch.c | 18 +++++------ .../replication/logical/reorderbuffer.c | 31 +++++++++---------- src/backend/storage/buffer/bufmgr.c | 11 +++---- src/include/lib/binaryheap.h | 14 ++++----- 7 files changed, 61 insertions(+), 65 deletions(-) diff --git a/src/backend/executor/nodeGatherMerge.c b/src/backend/executor/nodeGatherMerge.c index 9d5e1a46e9..021682d87c 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(const void *a, const 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); @@ -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)); + (void *) (intptr_t) i); } else { @@ -565,10 +565,10 @@ 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)); + i = (intptr_t) binaryheap_first(gm_state->gm_heap); if (gather_merge_readnext(gm_state, i, false)) - binaryheap_replace_first(gm_state->gm_heap, Int32GetDatum(i)); + binaryheap_replace_first(gm_state->gm_heap, (void *) (intptr_t) i); else { /* reader exhausted, remove it from heap */ @@ -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)); + i = (intptr_t) binaryheap_first(gm_state->gm_heap); 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(const void *a, const void *b, void *arg) { GatherMergeState *node = (GatherMergeState *) arg; - SlotNumber slot1 = DatumGetInt32(a); - SlotNumber slot2 = DatumGetInt32(b); + SlotNumber slot1 = (intptr_t) a; + SlotNumber slot2 = (intptr_t) 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..b3e2c29401 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(const void *a, const void *b, void *arg); /* ---------------------------------------------------------------- @@ -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, (void *) (intptr_t) i); } binaryheap_build(node->ms_heap); node->ms_initialized = true; @@ -244,10 +244,10 @@ 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)); + i = (intptr_t) binaryheap_first(node->ms_heap); 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, (void *) (intptr_t) i); else (void) binaryheap_remove_first(node->ms_heap); } @@ -259,7 +259,7 @@ ExecMergeAppend(PlanState *pstate) } else { - i = DatumGetInt32(binaryheap_first(node->ms_heap)); + i = (intptr_t) binaryheap_first(node->ms_heap); 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(const void *a, const void *b, void *arg) { MergeAppendState *node = (MergeAppendState *) arg; - SlotNumber slot1 = DatumGetInt32(a); - SlotNumber slot2 = DatumGetInt32(b); + SlotNumber slot1 = (intptr_t) a; + SlotNumber slot2 = (intptr_t) 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..c7fcfc550b 100644 --- a/src/backend/lib/binaryheap.c +++ b/src/backend/lib/binaryheap.c @@ -34,7 +34,7 @@ binaryheap_allocate(int capacity, binaryheap_comparator compare, void *arg) int sz; binaryheap *heap; - sz = offsetof(binaryheap, bh_nodes) + sizeof(Datum) * capacity; + sz = offsetof(binaryheap, bh_nodes) + sizeof(void *) * capacity; heap = (binaryheap *) palloc(sz); heap->bh_space = capacity; heap->bh_compare = compare; @@ -106,7 +106,7 @@ 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"); @@ -138,7 +138,7 @@ 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"); @@ -154,7 +154,7 @@ 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 +void * binaryheap_first(binaryheap *heap) { Assert(!binaryheap_empty(heap) && heap->bh_has_heap_property); @@ -169,10 +169,10 @@ binaryheap_first(binaryheap *heap) * that this routine is not used on an empty heap. O(log n) worst * case. */ -Datum +void * binaryheap_remove_first(binaryheap *heap) { - Datum result; + void *result; Assert(!binaryheap_empty(heap) && heap->bh_has_heap_property); @@ -204,7 +204,7 @@ 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); @@ -221,7 +221,7 @@ binaryheap_replace_first(binaryheap *heap, Datum d) static void sift_up(binaryheap *heap, int node_off) { - Datum node_val = heap->bh_nodes[node_off]; + void *node_val = heap->bh_nodes[node_off]; /* * Within the loop, the node_off'th array entry is a "hole" that @@ -232,7 +232,7 @@ 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 @@ -264,7 +264,7 @@ sift_up(binaryheap *heap, int node_off) static void sift_down(binaryheap *heap, int node_off) { - Datum node_val = heap->bh_nodes[node_off]; + void *node_val = heap->bh_nodes[node_off]; /* * Within the loop, the node_off'th array entry is a "hole" that diff --git a/src/backend/postmaster/pgarch.c b/src/backend/postmaster/pgarch.c index 46af349564..9491b822db 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(const void *a, const void *b, void *arg); static void LoadArchiveLibrary(void); static void pgarch_call_module_shutdown_cb(int code, Datum arg); @@ -631,22 +631,22 @@ 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) + basename, NULL) > 0) { /* * Remove the lowest priority file and add the current one to the * heap. */ - arch_file = DatumGetCString(binaryheap_remove_first(arch_files->arch_heap)); + arch_file = binaryheap_remove_first(arch_files->arch_heap); strcpy(arch_file, basename); - binaryheap_add(arch_files->arch_heap, CStringGetDatum(arch_file)); + binaryheap_add(arch_files->arch_heap, arch_file); } } FreeDir(rldir); @@ -668,7 +668,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)); + arch_files->arch_files[i] = binaryheap_remove_first(arch_files->arch_heap); /* Return the highest priority file. */ arch_files->arch_files_size--; @@ -686,10 +686,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(const void *a, const void *b, void *arg) { - char *a_str = DatumGetCString(a); - char *b_str = DatumGetCString(b); + const char *a_str = (const char *) a; + const char *b_str = (const 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 26d252bd87..191c0d1181 100644 --- a/src/backend/replication/logical/reorderbuffer.c +++ b/src/backend/replication/logical/reorderbuffer.c @@ -1223,11 +1223,10 @@ ReorderBufferCommitChild(ReorderBuffer *rb, TransactionId xid, * Binary heap comparison function. */ static int -ReorderBufferIterCompare(Datum a, Datum b, void *arg) +ReorderBufferIterCompare(const void *a, const 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 = ((ReorderBufferIterTXNEntry *) a)->lsn; + XLogRecPtr pos_b = ((ReorderBufferIterTXNEntry *) b)->lsn; if (pos_a < pos_b) return 1; @@ -1298,7 +1297,7 @@ ReorderBufferIterTXNInit(ReorderBuffer *rb, ReorderBufferTXN *txn, /* allocate heap */ state->heap = binaryheap_allocate(state->nr_txns, ReorderBufferIterCompare, - state); + NULL); /* Now that the state fields are initialized, it is safe to return it. */ *iter_state = state; @@ -1330,7 +1329,7 @@ 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, &state->entries[off++]); } /* add subtransactions if they contain changes */ @@ -1359,7 +1358,7 @@ 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, &state->entries[off++]); } } @@ -1378,14 +1377,12 @@ ReorderBufferIterTXNNext(ReorderBuffer *rb, ReorderBufferIterTXNState *state) { ReorderBufferChange *change; ReorderBufferIterTXNEntry *entry; - int32 off; /* nothing there anymore */ if (state->heap->bh_size == 0) return NULL; - off = DatumGetInt32(binaryheap_first(state->heap)); - entry = &state->entries[off]; + entry = binaryheap_first(state->heap); /* free memory we might have "leaked" in the previous *Next call */ if (!dlist_is_empty(&state->old_change)) @@ -1411,10 +1408,10 @@ ReorderBufferIterTXNNext(ReorderBuffer *rb, ReorderBufferIterTXNState *state) dlist_container(ReorderBufferChange, node, next); /* txn stays the same */ - state->entries[off].lsn = next_change->lsn; - state->entries[off].change = next_change; + entry->lsn = next_change->lsn; + entry->change = next_change; - binaryheap_replace_first(state->heap, Int32GetDatum(off)); + binaryheap_replace_first(state->heap, entry); return change; } @@ -1435,7 +1432,7 @@ ReorderBufferIterTXNNext(ReorderBuffer *rb, ReorderBufferIterTXNState *state) */ rb->totalBytes += entry->txn->size; if (ReorderBufferRestoreChanges(rb, entry->txn, &entry->file, - &state->entries[off].segno)) + &entry->segno)) { /* successfully restored changes from disk */ ReorderBufferChange *next_change = @@ -1448,9 +1445,9 @@ ReorderBufferIterTXNNext(ReorderBuffer *rb, ReorderBufferIterTXNState *state) Assert(entry->txn->nentries_mem); /* txn stays the same */ - state->entries[off].lsn = next_change->lsn; - state->entries[off].change = next_change; - binaryheap_replace_first(state->heap, Int32GetDatum(off)); + entry->lsn = next_change->lsn; + entry->change = next_change; + binaryheap_replace_first(state->heap, entry); return change; } diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c index a7e3b9bb1d..391d5f3dd8 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(const void *a, const void *b, void *arg); /* @@ -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,7 @@ BufferSync(int flags) while (!binaryheap_empty(ts_heap)) { BufferDesc *bufHdr = NULL; - CkptTsStatus *ts_stat = (CkptTsStatus *) - DatumGetPointer(binaryheap_first(ts_heap)); + CkptTsStatus *ts_stat = (CkptTsStatus *) binaryheap_first(ts_heap); buf_id = CkptBufferIds[ts_stat->index].buf_id; Assert(buf_id != -1); @@ -2713,7 +2712,7 @@ BufferSync(int flags) else { /* update heap with the new progress */ - binaryheap_replace_first(ts_heap, PointerGetDatum(ts_stat)); + binaryheap_replace_first(ts_heap, ts_stat); } /* @@ -5416,7 +5415,7 @@ 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(const void *a, const void *b, void *arg) { CkptTsStatus *sa = (CkptTsStatus *) a; CkptTsStatus *sb = (CkptTsStatus *) b; diff --git a/src/include/lib/binaryheap.h b/src/include/lib/binaryheap.h index 52f7b06b25..71a25db0f4 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) (const void *a, const void *b, void *arg); /* * binaryheap @@ -34,7 +34,7 @@ typedef struct binaryheap bool bh_has_heap_property; /* debugging cross-check */ binaryheap_comparator bh_compare; void *bh_arg; - Datum bh_nodes[FLEXIBLE_ARRAY_MEMBER]; + void *bh_nodes[FLEXIBLE_ARRAY_MEMBER]; } binaryheap; extern binaryheap *binaryheap_allocate(int capacity, @@ -42,12 +42,12 @@ extern binaryheap *binaryheap_allocate(int capacity, 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); +extern void *binaryheap_remove_first(binaryheap *heap); +extern void binaryheap_replace_first(binaryheap *heap, void *d); #define binaryheap_empty(h) ((h)->bh_size == 0) -- 2.25.1