public inbox for [email protected]  
help / color / mirror / Atom feed
[PATCH v6 3/3] Optimize hash function
2+ messages / 2 participants
[nested] [flat]

* [PATCH v6 3/3] Optimize hash function
@ 2021-01-20 22:06  Heikki Linnakangas <[email protected]>
  0 siblings, 0 replies; 2+ messages in thread

From: Heikki Linnakangas @ 2021-01-20 22:06 UTC (permalink / raw)

---
 src/backend/utils/resowner/resowner.c | 24 ++++++++++++++++++------
 src/include/common/hashfn.h           | 15 +++++++++++++++
 2 files changed, 33 insertions(+), 6 deletions(-)

diff --git a/src/backend/utils/resowner/resowner.c b/src/backend/utils/resowner/resowner.c
index 1db1e7cf14b..72e4d8a784c 100644
--- a/src/backend/utils/resowner/resowner.c
+++ b/src/backend/utils/resowner/resowner.c
@@ -163,15 +163,27 @@ static void ReleaseAuxProcessResourcesCallback(int code, Datum arg);
  *	  INTERNAL ROUTINES														 *
  *****************************************************************************/
 
+/*
+ * Hash function for value+kind combination.
+ */
 static inline uint32
 hash_resource_elem(Datum value, ResourceOwnerFuncs *kind)
 {
-	Datum		data[2];
-
-	data[0] = value;
-	data[1] = PointerGetDatum(kind);
-
-	return hash_bytes((unsigned char *) &data, 2 * SIZEOF_DATUM);
+	/*
+	 * Most resources types store a pointer in 'value', and pointers are
+	 * unique all on their own. But some resources store plain integers (Files
+	 * and Buffers as of this writing), so we want to incorporate the 'kind'
+	 * in the hash too, otherwise those resources will collide a lot. But
+	 * because there are only a few resource kinds like that - and only a few
+	 * resource kinds to begin with - we don't need to work too hard to mix
+	 * 'kind' into the hash. Just add it with hash_combine(), it perturbs the
+	 * result enough for our purposes.
+	 */
+#if SIZEOF_DATUM == 8
+	return hash_combine64(murmurhash64((uint64) value), (uint64) kind);
+#else
+	return hash_combine(murmurhash32((uint32) value), (uint32) kind);
+#endif
 }
 
 static void
diff --git a/src/include/common/hashfn.h b/src/include/common/hashfn.h
index c634cc067a1..009ffbbdd38 100644
--- a/src/include/common/hashfn.h
+++ b/src/include/common/hashfn.h
@@ -101,4 +101,19 @@ murmurhash32(uint32 data)
 	return h;
 }
 
+/* 64-bit variant */
+static inline uint64
+murmurhash64(uint64 data)
+{
+	uint64		h = data;
+
+	h ^= h >> 33;
+	h *= 0xff51afd7ed558ccd;
+	h ^= h >> 33;
+	h *= 0xc4ceb9fe1a85ec53;
+	h ^= h >> 33;
+
+	return h;
+}
+
 #endif							/* HASHFN_H */
-- 
2.29.2


--------------F30A5443A5D524A5CBC51AE7
Content-Type: text/x-patch; charset=UTF-8;
 name="v2-0001-resownerbench.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
 filename="v2-0001-resownerbench.patch"



^ permalink  raw  reply  [nested|flat] 2+ messages in thread

* Re: Warning in geqo_main.c from clang 13
@ 2022-01-22 22:34  Tom Lane <[email protected]>
  0 siblings, 0 replies; 2+ messages in thread

From: Tom Lane @ 2022-01-22 22:34 UTC (permalink / raw)
  To: Thomas Munro <[email protected]>; +Cc: pgsql-hackers

I wrote:
> Thomas Munro <[email protected]> writes:
>> Clang 13 on my machine and peripatus (but not Apple clang 13 on eg
>> sifika, I'm still confused about Apple's versioning but I think that's
>> really llvm 12-based) warns:
>> geqo_main.c:86:8: warning: variable 'edge_failures' set but not used
>> [-Wunused-but-set-variable]
>> Here's one way to silence it.

> I'm kind of inclined to just drop the edge_failures recording/logging
> altogether, rather than make that rats-nest of #ifdefs even worse.
> It's not like anyone has cared about that number in the last decade
> or two.

We're starting to see more buildfarm animals producing this warning,
so I took another look, and thought of a slightly less invasive way to
silence it.  I confirmed this works with clang 13.0.0 on Fedora 35.

			regards, tom lane



Attachments:

  [text/x-diff] silence-clang-warning-v2.patch (753B, ../../[email protected]/2-silence-clang-warning-v2.patch)
  download | inline diff:
diff --git a/src/backend/optimizer/geqo/geqo_main.c b/src/backend/optimizer/geqo/geqo_main.c
index 333a26a695..68ed74325c 100644
--- a/src/backend/optimizer/geqo/geqo_main.c
+++ b/src/backend/optimizer/geqo/geqo_main.c
@@ -227,12 +227,17 @@ geqo(PlannerInfo *root, int number_of_rels, List *initial_rels)
 	}
 
 
-#if defined(ERX) && defined(GEQO_DEBUG)
+#if defined(ERX)
+#if defined(GEQO_DEBUG)
 	if (edge_failures != 0)
 		elog(LOG, "[GEQO] failures: %d, average: %d",
 			 edge_failures, (int) number_generations / edge_failures);
 	else
 		elog(LOG, "[GEQO] no edge failures detected");
+#else
+	/* suppress variable-set-but-not-used warnings from some compilers */
+	(void) edge_failures;
+#endif
 #endif
 
 #if defined(CX) && defined(GEQO_DEBUG)


^ permalink  raw  reply  [nested|flat] 2+ messages in thread


end of thread, other threads:[~2022-01-22 22:34 UTC | newest]

Thread overview: 2+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2021-01-20 22:06 [PATCH v6 3/3] Optimize hash function Heikki Linnakangas <[email protected]>
2022-01-22 22:34 Re: Warning in geqo_main.c from clang 13 Tom Lane <[email protected]>

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox