Revert indexed and enlargable binary heap implementation.

This reverts commit b840508644 and bcb14f4abc. These commits were made
for commit 5bec1d6bc5 (Improve eviction algorithm in ReorderBuffer
using max-heap for many subtransactions). However, per discussion,
commit efb8acc0d0 replaced binary heap + index with pairing heap, and
made these commits unnecessary.

Reported-by: Jeff Davis
Discussion: https://postgr.es/m/12747c15811d94efcc5cda72d6b35c80d7bf3443.camel%40j-davis.com

src/backend/executor/nodeGatherMerge.c

@@ -422,7 +422,6 @@ gather_merge_setup(GatherMergeState *gm_state)
 	/* Allocate the resources for the merge */
 	gm_state->gm_heap = binaryheap_allocate(nreaders + 1,
 											heap_compare_slots,
-											false,
 											gm_state);
 }
 

src/backend/executor/nodeMergeAppend.c

@@ -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, false,
+	mergestate->ms_heap = binaryheap_allocate(nplans, heap_compare_slots,
 											  mergestate);
 
 	/*

src/backend/postmaster/pgarch.c

@@ -258,8 +258,7 @@ PgArchiverMain(char *startup_data, size_t startup_data_len)
 
 	/* Initialize our max-heap for prioritizing files to archive. */
 	arch_files->arch_heap = binaryheap_allocate(NUM_FILES_PER_DIRECTORY_SCAN,
-												ready_file_comparator, false,
+												ready_file_comparator, NULL);
-												NULL);
 
 	/* Initialize our memory context. */
 	archive_context = AllocSetContextCreate(TopMemoryContext,

src/backend/replication/logical/reorderbuffer.c

@@ -1303,7 +1303,6 @@ ReorderBufferIterTXNInit(ReorderBuffer *rb, ReorderBufferTXN *txn,
 	/* allocate heap */
 	state->heap = binaryheap_allocate(state->nr_txns,
 									  ReorderBufferIterCompare,
-									  false,
 									  state);
 
 	/* Now that the state fields are initialized, it is safe to return it. */

src/backend/storage/buffer/bufmgr.c

@@ -3014,7 +3014,6 @@ BufferSync(int flags)
 	 */
 	ts_heap = binaryheap_allocate(num_spaces,
 								  ts_ckpt_progress_comparator,
-								  false,
 								  NULL);
 
 	for (i = 0; i < num_spaces; i++)

src/bin/pg_dump/pg_backup_archiver.c

@@ -4200,7 +4200,6 @@ restore_toc_entries_parallel(ArchiveHandle *AH, ParallelState *pstate,
 	/* Set up ready_heap with enough room for all known TocEntrys */
 	ready_heap = binaryheap_allocate(AH->tocCount,
 									 TocEntrySizeCompareBinaryheap,
-									 false,
 									 NULL);
 
 	/*

src/bin/pg_dump/pg_dump_sort.c

@@ -405,7 +405,7 @@ TopoSort(DumpableObject **objs,
 		return true;
 
 	/* Create workspace for the above-described heap */
-	pendingHeap = binaryheap_allocate(numObjs, int_cmp, false, NULL);
+	pendingHeap = binaryheap_allocate(numObjs, int_cmp, NULL);
 
 	/*
 	 * Scan the constraints, and for each item in the input, generate a count

src/common/binaryheap.c

@@ -22,70 +22,33 @@
 #ifdef FRONTEND
 #include "common/logging.h"
 #endif
-#include "common/hashfn.h"
 #include "lib/binaryheap.h"
 
-/*
- * Define parameters for hash table code generation. The interface is *also*
- * declared in binaryheap.h (to generate the types, which are externally
- * visible).
- */
-#define SH_PREFIX bh_nodeidx
-#define SH_ELEMENT_TYPE bh_nodeidx_entry
-#define SH_KEY_TYPE bh_node_type
-#define SH_KEY key
-#define SH_HASH_KEY(tb, key) \
-	hash_bytes((const unsigned char *) &key, sizeof(bh_node_type))
-#define SH_EQUAL(tb, a, b) (memcmp(&a, &b, sizeof(bh_node_type)) == 0)
-#define SH_SCOPE extern
-#ifdef FRONTEND
-#define SH_RAW_ALLOCATOR pg_malloc0
-#endif
-#define SH_STORE_HASH
-#define SH_GET_HASH(tb, a) a->hash
-#define SH_DEFINE
-#include "lib/simplehash.h"
-
 static void sift_down(binaryheap *heap, int node_off);
 static void sift_up(binaryheap *heap, int node_off);
 
 /*
  * binaryheap_allocate
  *
- * Returns a pointer to a newly-allocated heap with the given initial number
+ * Returns a pointer to a newly-allocated heap that has the capacity to
- * of nodes, and with the heap property defined by the given comparator
+ * store the given number of nodes, with the heap property defined by
- * function, which will be invoked with the additional argument specified by
+ * the given comparator function, which will be invoked with the additional
- * 'arg'.
+ * argument specified by 'arg'.
- *
- * If 'indexed' is true, we create a hash table to track each node's
- * index in the heap, enabling to perform some operations such as
- * binaryheap_remove_node_ptr() etc.
  */
 binaryheap *
-binaryheap_allocate(int num_nodes, binaryheap_comparator compare,
+binaryheap_allocate(int capacity, binaryheap_comparator compare, void *arg)
-					bool indexed, void *arg)
 {
+	int			sz;
 	binaryheap *heap;
 
-	heap = (binaryheap *) palloc(sizeof(binaryheap));
+	sz = offsetof(binaryheap, bh_nodes) + sizeof(bh_node_type) * capacity;
-	heap->bh_space = num_nodes;
+	heap = (binaryheap *) palloc(sz);
+	heap->bh_space = capacity;
 	heap->bh_compare = compare;
 	heap->bh_arg = arg;
 
 	heap->bh_size = 0;
 	heap->bh_has_heap_property = true;
-	heap->bh_nodes = (bh_node_type *) palloc(sizeof(bh_node_type) * num_nodes);
-	heap->bh_nodeidx = NULL;
-
-	if (indexed)
-	{
-#ifdef FRONTEND
-		heap->bh_nodeidx = bh_nodeidx_create(num_nodes, NULL);
-#else
-		heap->bh_nodeidx = bh_nodeidx_create(CurrentMemoryContext, num_nodes,
-											 NULL);
-#endif
-	}
 
 	return heap;
 }
@@ -101,9 +64,6 @@ binaryheap_reset(binaryheap *heap)
 {
 	heap->bh_size = 0;
 	heap->bh_has_heap_property = true;
-
-	if (binaryheap_indexed(heap))
-		bh_nodeidx_reset(heap->bh_nodeidx);
 }
 
 /*
@@ -114,10 +74,6 @@ binaryheap_reset(binaryheap *heap)
 void
 binaryheap_free(binaryheap *heap)
 {
-	if (binaryheap_indexed(heap))
-		bh_nodeidx_destroy(heap->bh_nodeidx);
-
-	pfree(heap->bh_nodes);
 	pfree(heap);
 }
 
@@ -148,78 +104,6 @@ parent_offset(int i)
 	return (i - 1) / 2;
 }
 
-/*
- * Double the space allocated for nodes.
- */
-static void
-enlarge_node_array(binaryheap *heap)
-{
-	heap->bh_space *= 2;
-	heap->bh_nodes = repalloc(heap->bh_nodes,
-							  sizeof(bh_node_type) * heap->bh_space);
-}
-
-/*
- * Set the given node at the 'index' and track it if required.
- *
- * Return true if the node's index is already tracked.
- */
-static bool
-set_node(binaryheap *heap, bh_node_type node, int index)
-{
-	bool		found = false;
-
-	/* Set the node to the nodes array */
-	heap->bh_nodes[index] = node;
-
-	if (binaryheap_indexed(heap))
-	{
-		bh_nodeidx_entry *ent;
-
-		/* Keep track of the node index */
-		ent = bh_nodeidx_insert(heap->bh_nodeidx, node, &found);
-		ent->index = index;
-	}
-
-	return found;
-}
-
-/*
- * Remove the node's index from the hash table if the heap is indexed.
- */
-static inline void
-delete_nodeidx(binaryheap *heap, bh_node_type node)
-{
-	if (binaryheap_indexed(heap))
-		bh_nodeidx_delete(heap->bh_nodeidx, node);
-}
-
-/*
- * Replace the existing node at 'idx' with the given 'new_node'. Also
- * update their positions accordingly. Note that we assume the new_node's
- * position is already tracked if enabled, i.e. the new_node is already
- * present in the heap.
- */
-static void
-replace_node(binaryheap *heap, int index, bh_node_type new_node)
-{
-	bool		found PG_USED_FOR_ASSERTS_ONLY;
-
-	/* Quick return if not necessary to move */
-	if (heap->bh_nodes[index] == new_node)
-		return;
-
-	/* Remove the overwritten node's index */
-	delete_nodeidx(heap, heap->bh_nodes[index]);
-
-	/*
-	 * Replace it with the given new node. This node's position must also be
-	 * tracked as we assume to replace the node with the existing node.
-	 */
-	found = set_node(heap, new_node, index);
-	Assert(!binaryheap_indexed(heap) || found);
-}
-
 /*
  * binaryheap_add_unordered
  *
@@ -231,12 +115,16 @@ replace_node(binaryheap *heap, int index, bh_node_type new_node)
 void
 binaryheap_add_unordered(binaryheap *heap, bh_node_type d)
 {
-	/* make sure enough space for a new node */
 	if (heap->bh_size >= heap->bh_space)
-		enlarge_node_array(heap);
+	{
-
+#ifdef FRONTEND
+		pg_fatal("out of binary heap slots");
+#else
+		elog(ERROR, "out of binary heap slots");
+#endif
+	}
 	heap->bh_has_heap_property = false;
-	set_node(heap, d, heap->bh_size);
+	heap->bh_nodes[heap->bh_size] = d;
 	heap->bh_size++;
 }
 
@@ -265,11 +153,15 @@ binaryheap_build(binaryheap *heap)
 void
 binaryheap_add(binaryheap *heap, bh_node_type d)
 {
-	/* make sure enough space for a new node */
 	if (heap->bh_size >= heap->bh_space)
-		enlarge_node_array(heap);
+	{
-
+#ifdef FRONTEND
-	set_node(heap, d, heap->bh_size);
+		pg_fatal("out of binary heap slots");
+#else
+		elog(ERROR, "out of binary heap slots");
+#endif
+	}
+	heap->bh_nodes[heap->bh_size] = d;
 	heap->bh_size++;
 	sift_up(heap, heap->bh_size - 1);
 }
@@ -310,8 +202,6 @@ binaryheap_remove_first(binaryheap *heap)
 	if (heap->bh_size == 1)
 	{
 		heap->bh_size--;
-		delete_nodeidx(heap, result);
-
 		return result;
 	}
 
@@ -319,7 +209,7 @@ binaryheap_remove_first(binaryheap *heap)
 	 * Remove the last node, placing it in the vacated root entry, and sift
 	 * the new root node down to its correct position.
 	 */
-	replace_node(heap, 0, heap->bh_nodes[--heap->bh_size]);
+	heap->bh_nodes[0] = heap->bh_nodes[--heap->bh_size];
 	sift_down(heap, 0);
 
 	return result;
@@ -345,7 +235,7 @@ binaryheap_remove_node(binaryheap *heap, int n)
 						   heap->bh_arg);
 
 	/* remove the last node, placing it in the vacated entry */
-	replace_node(heap, n, heap->bh_nodes[heap->bh_size]);
+	heap->bh_nodes[n] = heap->bh_nodes[heap->bh_size];
 
 	/* sift as needed to preserve the heap property */
 	if (cmp > 0)
@@ -354,77 +244,6 @@ binaryheap_remove_node(binaryheap *heap, int n)
 		sift_down(heap, n);
 }
 
-/*
- * binaryheap_remove_node_ptr
- *
- * Similar to binaryheap_remove_node() but removes the given node. The caller
- * must ensure that the given node is in the heap. O(log n) worst case.
- *
- * This function can be used only if the heap is indexed.
- */
-void
-binaryheap_remove_node_ptr(binaryheap *heap, bh_node_type d)
-{
-	bh_nodeidx_entry *ent;
-
-	Assert(!binaryheap_empty(heap) && heap->bh_has_heap_property);
-	Assert(binaryheap_indexed(heap));
-
-	ent = bh_nodeidx_lookup(heap->bh_nodeidx, d);
-	Assert(ent);
-
-	binaryheap_remove_node(heap, ent->index);
-}
-
-/*
- * Workhorse for binaryheap_update_up and binaryheap_update_down.
- */
-static void
-resift_node(binaryheap *heap, bh_node_type node, bool sift_dir_up)
-{
-	bh_nodeidx_entry *ent;
-
-	Assert(!binaryheap_empty(heap) && heap->bh_has_heap_property);
-	Assert(binaryheap_indexed(heap));
-
-	ent = bh_nodeidx_lookup(heap->bh_nodeidx, node);
-	Assert(ent);
-	Assert(ent->index >= 0 && ent->index < heap->bh_size);
-
-	if (sift_dir_up)
-		sift_up(heap, ent->index);
-	else
-		sift_down(heap, ent->index);
-}
-
-/*
- * binaryheap_update_up
- *
- * Sift the given node up after the node's key is updated. The caller must
- * ensure that the given node is in the heap. O(log n) worst case.
- *
- * This function can be used only if the heap is indexed.
- */
-void
-binaryheap_update_up(binaryheap *heap, bh_node_type d)
-{
-	resift_node(heap, d, true);
-}
-
-/*
- * binaryheap_update_down
- *
- * Sift the given node down after the node's key is updated. The caller must
- * ensure that the given node is in the heap. O(log n) worst case.
- *
- * This function can be used only if the heap is indexed.
- */
-void
-binaryheap_update_down(binaryheap *heap, bh_node_type d)
-{
-	resift_node(heap, d, false);
-}
-
 /*
  * binaryheap_replace_first
  *
@@ -437,7 +256,7 @@ binaryheap_replace_first(binaryheap *heap, bh_node_type d)
 {
 	Assert(!binaryheap_empty(heap) && heap->bh_has_heap_property);
 
-	replace_node(heap, 0, d);
+	heap->bh_nodes[0] = d;
 
 	if (heap->bh_size > 1)
 		sift_down(heap, 0);
@@ -479,11 +298,11 @@ 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.
 		 */
-		set_node(heap, parent_val, node_off);
+		heap->bh_nodes[node_off] = parent_val;
 		node_off = parent_off;
 	}
 	/* Re-fill the hole */
-	set_node(heap, node_val, node_off);
+	heap->bh_nodes[node_off] = node_val;
 }
 
 /*
@@ -538,9 +357,9 @@ 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.
 		 */
-		set_node(heap, heap->bh_nodes[swap_off], node_off);
+		heap->bh_nodes[node_off] = heap->bh_nodes[swap_off];
 		node_off = swap_off;
 	}
 	/* Re-fill the hole */
-	set_node(heap, node_val, node_off);
+	heap->bh_nodes[node_off] = node_val;
 }

src/include/lib/binaryheap.h

@@ -29,29 +29,6 @@ typedef Datum bh_node_type;
  */
 typedef int (*binaryheap_comparator) (bh_node_type a, bh_node_type b, void *arg);
 
-/*
- * Struct for a hash table element to store the node's index in the bh_nodes
- * array.
- */
-typedef struct bh_nodeidx_entry
-{
-	bh_node_type key;
-	int			index;			/* entry's index within the node array */
-	char		status;			/* hash status */
-	uint32		hash;			/* hash values (cached) */
-} bh_nodeidx_entry;
-
-/* Define parameters necessary to generate the hash table interface. */
-#define SH_PREFIX bh_nodeidx
-#define SH_ELEMENT_TYPE bh_nodeidx_entry
-#define SH_KEY_TYPE bh_node_type
-#define SH_SCOPE extern
-#ifdef FRONTEND
-#define SH_RAW_ALLOCATOR pg_malloc0
-#endif
-#define SH_DECLARE
-#include "lib/simplehash.h"
-
 /*
  * binaryheap
  *
@@ -69,19 +46,12 @@ typedef struct binaryheap
 	bool		bh_has_heap_property;	/* debugging cross-check */
 	binaryheap_comparator bh_compare;
 	void	   *bh_arg;
-	bh_node_type *bh_nodes;
+	bh_node_type bh_nodes[FLEXIBLE_ARRAY_MEMBER];
-
-	/*
-	 * If bh_nodeidx is not NULL, the bh_nodeidx is used to track of each
-	 * node's index in bh_nodes. This enables the caller to perform
-	 * binaryheap_remove_node_ptr(), binaryheap_update_up/down in O(log n).
-	 */
-	bh_nodeidx_hash *bh_nodeidx;
 } binaryheap;
 
-extern binaryheap *binaryheap_allocate(int num_nodes,
+extern binaryheap *binaryheap_allocate(int capacity,
 									   binaryheap_comparator compare,
-									   bool indexed, void *arg);
+									   void *arg);
 extern void binaryheap_reset(binaryheap *heap);
 extern void binaryheap_free(binaryheap *heap);
 extern void binaryheap_add_unordered(binaryheap *heap, bh_node_type d);
@@ -90,14 +60,10 @@ extern void binaryheap_add(binaryheap *heap, bh_node_type d);
 extern bh_node_type binaryheap_first(binaryheap *heap);
 extern bh_node_type binaryheap_remove_first(binaryheap *heap);
 extern void binaryheap_remove_node(binaryheap *heap, int n);
-extern void binaryheap_remove_node_ptr(binaryheap *heap, bh_node_type d);
 extern void binaryheap_replace_first(binaryheap *heap, bh_node_type d);
-extern void binaryheap_update_up(binaryheap *heap, bh_node_type d);
-extern void binaryheap_update_down(binaryheap *heap, bh_node_type d);
 
 #define binaryheap_empty(h)			((h)->bh_size == 0)
 #define binaryheap_size(h)			((h)->bh_size)
 #define binaryheap_get_node(h, n)	((h)->bh_nodes[n])
-#define binaryheap_indexed(h)		((h)->bh_nodeidx != NULL)
 
 #endif							/* BINARYHEAP_H */

src/tools/pgindent/typedefs.list

@@ -4125,4 +4125,3 @@ TidStoreIter
 TidStoreIterResult
 BlocktableEntry
 ItemArray
-bh_nodeidx_entry