[PATCH v7 3/3] Optimize hash function

($INBOX_DIR/description missing)  
help / color / mirror / Atom feed

[PATCH v7 3/3] Optimize hash function
51+ messages / 3 participants
[nested] [flat]

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

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

* [PATCH v7 3/3] Optimize hash function
@ 2021-01-20 22:06 Heikki Linnakangas <[email protected]>
  0 siblings, 0 replies; 51+ 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 d62b1495dd3..6189d051072 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.30.1


--------------C6A03312D0F99F9183A0DF46--





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

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

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

* [PATCH v7 3/3] Optimize hash function
@ 2021-01-20 22:06 Heikki Linnakangas <[email protected]>
  0 siblings, 0 replies; 51+ 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 d62b1495dd3..6189d051072 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.30.1


--------------C6A03312D0F99F9183A0DF46--





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

* [PATCH v7 3/3] Optimize hash function
@ 2021-01-20 22:06 Heikki Linnakangas <[email protected]>
  0 siblings, 0 replies; 51+ 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 d62b1495dd3..6189d051072 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.30.1


--------------C6A03312D0F99F9183A0DF46--





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

* [PATCH v7 3/3] Optimize hash function
@ 2021-01-20 22:06 Heikki Linnakangas <[email protected]>
  0 siblings, 0 replies; 51+ 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 d62b1495dd3..6189d051072 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.30.1


--------------C6A03312D0F99F9183A0DF46--





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

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

* [PATCH v7 3/3] Optimize hash function
@ 2021-01-20 22:06 Heikki Linnakangas <[email protected]>
  0 siblings, 0 replies; 51+ 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 d62b1495dd3..6189d051072 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.30.1


--------------C6A03312D0F99F9183A0DF46--





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

* [PATCH v7 3/3] Optimize hash function
@ 2021-01-20 22:06 Heikki Linnakangas <[email protected]>
  0 siblings, 0 replies; 51+ 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 d62b1495dd3..6189d051072 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.30.1


--------------C6A03312D0F99F9183A0DF46--





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

* [PATCH v7 3/3] Optimize hash function
@ 2021-01-20 22:06 Heikki Linnakangas <[email protected]>
  0 siblings, 0 replies; 51+ 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 d62b1495dd3..6189d051072 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.30.1


--------------C6A03312D0F99F9183A0DF46--





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

* [PATCH v7 3/3] Optimize hash function
@ 2021-01-20 22:06 Heikki Linnakangas <[email protected]>
  0 siblings, 0 replies; 51+ 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 d62b1495dd3..6189d051072 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.30.1


--------------C6A03312D0F99F9183A0DF46--





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

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

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

* [PATCH v7 3/3] Optimize hash function
@ 2021-01-20 22:06 Heikki Linnakangas <[email protected]>
  0 siblings, 0 replies; 51+ 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 d62b1495dd3..6189d051072 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.30.1


--------------C6A03312D0F99F9183A0DF46--





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

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

* [PATCH v7 3/3] Optimize hash function
@ 2021-01-20 22:06 Heikki Linnakangas <[email protected]>
  0 siblings, 0 replies; 51+ 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 d62b1495dd3..6189d051072 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.30.1


--------------C6A03312D0F99F9183A0DF46--





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

* [PATCH v7 3/3] Optimize hash function
@ 2021-01-20 22:06 Heikki Linnakangas <[email protected]>
  0 siblings, 0 replies; 51+ 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 d62b1495dd3..6189d051072 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.30.1


--------------C6A03312D0F99F9183A0DF46--





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

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

* [PATCH v7 3/3] Optimize hash function
@ 2021-01-20 22:06 Heikki Linnakangas <[email protected]>
  0 siblings, 0 replies; 51+ 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 d62b1495dd3..6189d051072 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.30.1


--------------C6A03312D0F99F9183A0DF46--





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

* [PATCH v7 3/3] Optimize hash function
@ 2021-01-20 22:06 Heikki Linnakangas <[email protected]>
  0 siblings, 0 replies; 51+ 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 d62b1495dd3..6189d051072 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.30.1


--------------C6A03312D0F99F9183A0DF46--





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

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

* [PATCH v7 3/3] Optimize hash function
@ 2021-01-20 22:06 Heikki Linnakangas <[email protected]>
  0 siblings, 0 replies; 51+ 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 d62b1495dd3..6189d051072 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.30.1


--------------C6A03312D0F99F9183A0DF46--





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

* [PATCH v7 3/3] Optimize hash function
@ 2021-01-20 22:06 Heikki Linnakangas <[email protected]>
  0 siblings, 0 replies; 51+ 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 d62b1495dd3..6189d051072 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.30.1


--------------C6A03312D0F99F9183A0DF46--





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

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

* [PATCH v7 3/3] Optimize hash function
@ 2021-01-20 22:06 Heikki Linnakangas <[email protected]>
  0 siblings, 0 replies; 51+ 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 d62b1495dd3..6189d051072 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.30.1


--------------C6A03312D0F99F9183A0DF46--





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

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

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

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

* [PATCH v7 3/3] Optimize hash function
@ 2021-01-20 22:06 Heikki Linnakangas <[email protected]>
  0 siblings, 0 replies; 51+ 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 d62b1495dd3..6189d051072 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.30.1


--------------C6A03312D0F99F9183A0DF46--





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

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

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

* [PATCH v7 3/3] Optimize hash function
@ 2021-01-20 22:06 Heikki Linnakangas <[email protected]>
  0 siblings, 0 replies; 51+ 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 d62b1495dd3..6189d051072 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.30.1


--------------C6A03312D0F99F9183A0DF46--





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

* [PATCH v7 3/3] Optimize hash function
@ 2021-01-20 22:06 Heikki Linnakangas <[email protected]>
  0 siblings, 0 replies; 51+ 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 d62b1495dd3..6189d051072 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.30.1


--------------C6A03312D0F99F9183A0DF46--





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

* [PATCH v7 3/3] Optimize hash function
@ 2021-01-20 22:06 Heikki Linnakangas <[email protected]>
  0 siblings, 0 replies; 51+ 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 d62b1495dd3..6189d051072 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.30.1


--------------C6A03312D0F99F9183A0DF46--





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

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

* [PATCH v7 3/3] Optimize hash function
@ 2021-01-20 22:06 Heikki Linnakangas <[email protected]>
  0 siblings, 0 replies; 51+ 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 d62b1495dd3..6189d051072 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.30.1


--------------C6A03312D0F99F9183A0DF46--





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

* [PATCH v7 3/3] Optimize hash function
@ 2021-01-20 22:06 Heikki Linnakangas <[email protected]>
  0 siblings, 0 replies; 51+ 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 d62b1495dd3..6189d051072 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.30.1


--------------C6A03312D0F99F9183A0DF46--





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

* [PATCH v7 3/3] Optimize hash function
@ 2021-01-20 22:06 Heikki Linnakangas <[email protected]>
  0 siblings, 0 replies; 51+ 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 d62b1495dd3..6189d051072 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.30.1


--------------C6A03312D0F99F9183A0DF46--





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

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

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

* [PATCH v7 3/3] Optimize hash function
@ 2021-01-20 22:06 Heikki Linnakangas <[email protected]>
  0 siblings, 0 replies; 51+ 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 d62b1495dd3..6189d051072 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.30.1


--------------C6A03312D0F99F9183A0DF46--





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

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

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

* [PATCH v8 3/3] Optimize hash function
@ 2021-01-20 22:06 Heikki Linnakangas <[email protected]>
  0 siblings, 0 replies; 51+ 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 d62b1495dd3..6189d051072 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.30.2


--------------846F221A8A016B7AA8E567B3--





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

* [PATCH v10 3/3] Optimize hash function
@ 2021-10-18 10:59 Heikki Linnakangas <[email protected]>
  0 siblings, 0 replies; 51+ messages in thread

From: Heikki Linnakangas @ 2021-10-18 10:59 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 35e824d4534..118bd2b34c9 100644
--- a/src/backend/utils/resowner/resowner.c
+++ b/src/backend/utils/resowner/resowner.c
@@ -169,15 +169,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
 }
 
 /*
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.30.2


--------------AF226508B1039DE40796DE51--





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

* Re: gcc 12.1.0 warning
@ 2024-04-15 08:25 Nazir Bilal Yavuz <[email protected]>
  2024-04-23 16:59 ` Re: gcc 12.1.0 warning Andres Freund <[email protected]>
  0 siblings, 1 reply; 51+ messages in thread

From: Nazir Bilal Yavuz @ 2024-04-15 08:25 UTC (permalink / raw)
  To: Andres Freund <[email protected]>; +Cc: Erik Rijkers <[email protected]>; [email protected]; Thomas Munro <[email protected]>

Hi,

On Sat, 13 Apr 2024 at 05:25, Andres Freund <[email protected]> wrote:
>
> Hi,
>
> On 2023-10-27 13:09:01 +0300, Nazir Bilal Yavuz wrote:
> > I was testing 'upgrading CI Debian images to bookworm'. I tested the
> > bookworm on REL_15, REL_16 and upstream. REL_16 and upstream finished
> > successfully but the CompilerWarnings task failed on REL_15 with the
> > same error.
>
> Is that still the case?  I briefly tried to repro this outside of CI and
> couldn't reproduce the warning.
>
> I'd really like to upgrade the CI images, it doesn't seem great to continue
> using oldstable.
>
>
> > gcc version: 12.2.0
> >
> > CI Run: https://cirrus-ci.com/task/6151742664998912
>
> Unfortunately the logs aren't accessible anymore, so I can't check the precise
> patch level of the compiler and/or the precise invocation used.

I am able to reproduce this. I regenerated the debian bookworm image
and ran CI on REL_15_STABLE with this image.

CI Run: https://cirrus-ci.com/task/4978799442395136

--

Regards,
Nazir Bilal Yavuz
Microsoft






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

* Re: gcc 12.1.0 warning
  2024-04-15 08:25 Re: gcc 12.1.0 warning Nazir Bilal Yavuz <[email protected]>
@ 2024-04-23 16:59 ` Andres Freund <[email protected]>
  2024-05-10 09:13   ` Re: gcc 12.1.0 warning Nazir Bilal Yavuz <[email protected]>
  0 siblings, 1 reply; 51+ messages in thread

From: Andres Freund @ 2024-04-23 16:59 UTC (permalink / raw)
  To: Nazir Bilal Yavuz <[email protected]>; Tom Lane <[email protected]>; +Cc: Erik Rijkers <[email protected]>; [email protected]; Thomas Munro <[email protected]>

Hi,

On 2024-04-15 11:25:05 +0300, Nazir Bilal Yavuz wrote:
> I am able to reproduce this. I regenerated the debian bookworm image
> and ran CI on REL_15_STABLE with this image.
> 
> CI Run: https://cirrus-ci.com/task/4978799442395136

Hm, not sure why I wasn't able to repro - now I can.

It actually seems like a legitimate warning: The caller allocates the key as

static struct config_generic *
find_option(const char *name, bool create_placeholders, bool skip_errors,
			int elevel)
{
	const char **key = &name;

and then does
	res = (struct config_generic **) bsearch((void *) &key,
											 (void *) guc_variables,
											 num_guc_variables,
											 sizeof(struct config_generic *),
											 guc_var_compare);

while guc_var_compare() assume it's being passed a full config_generic:

static int
guc_var_compare(const void *a, const void *b)
{
	const struct config_generic *confa = *(struct config_generic *const *) a;
	const struct config_generic *confb = *(struct config_generic *const *) b;
	return guc_name_compare(confa->name, confb->name);
}


which several versions of gcc then complain about:

In function ‘guc_var_compare’,
    inlined from ‘bsearch’ at /usr/include/x86_64-linux-gnu/bits/stdlib-bsearch.h:33:23,
    inlined from ‘find_option’ at /home/andres/src/postgresql-15/src/backend/utils/misc/guc.c:5640:35:
/home/andres/src/postgresql-15/src/backend/utils/misc/guc.c:5727:38: warning: array subscript ‘const struct config_generic[0]’ is partly outside array bounds of ‘const char[8]’ [-Warray-bounds=]
 5727 |         return guc_name_compare(confa->name, confb->name);
      |                                 ~~~~~^~~~~~
/home/andres/src/postgresql-15/src/backend/utils/misc/guc.c: In function ‘find_option’:
/home/andres/src/postgresql-15/src/backend/utils/misc/guc.c:5627:25: note: object ‘name’ of size 8
 5627 | find_option(const char *name, bool create_placeholders, bool skip_errors,


Which seems entirely legitimate. ISTM that guc_var_compare() ought to only
cast the pointers to the key type, i.e. char *.  And incidentally that does
prevent the warning.

The reason it doesn't happen in newer versions of postgres is that we aren't
using guc_var_compare() in the relevant places anymore...

Greetings,

Andres Freund






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

* Re: gcc 12.1.0 warning
  2024-04-15 08:25 Re: gcc 12.1.0 warning Nazir Bilal Yavuz <[email protected]>
  2024-04-23 16:59 ` Re: gcc 12.1.0 warning Andres Freund <[email protected]>
@ 2024-05-10 09:13   ` Nazir Bilal Yavuz <[email protected]>
  2024-07-15 16:41     ` Re: gcc 12.1.0 warning Andres Freund <[email protected]>
  0 siblings, 1 reply; 51+ messages in thread

From: Nazir Bilal Yavuz @ 2024-05-10 09:13 UTC (permalink / raw)
  To: Andres Freund <[email protected]>; +Cc: Tom Lane <[email protected]>; Erik Rijkers <[email protected]>; [email protected]; Thomas Munro <[email protected]>

Hi,

On Tue, 23 Apr 2024 at 19:59, Andres Freund <[email protected]> wrote:
>
>
> Which seems entirely legitimate. ISTM that guc_var_compare() ought to only
> cast the pointers to the key type, i.e. char *.  And incidentally that does
> prevent the warning.
>
> The reason it doesn't happen in newer versions of postgres is that we aren't
> using guc_var_compare() in the relevant places anymore...

The fix is attached. It cleanly applies from REL_15_STABLE to
REL_12_STABLE, fixes the warnings and the tests pass.

-- 
Regards,
Nazir Bilal Yavuz
Microsoft


Attachments:

  [text/x-patch] v1-0001-Fix-Warray-bounds-warning-in-guc_var_compare-func.patch (1.5K, ../../CAN55FZ0o9wqVoMTh_gJCmj_+4XbX9VXzQF8OySPZ0R1saxV3bA@mail.gmail.com/2-v1-0001-Fix-Warray-bounds-warning-in-guc_var_compare-func.patch)
  download | inline diff:
From 588c99f5c402fc41414702133636e5a51f9e3470 Mon Sep 17 00:00:00 2001
From: Nazir Bilal Yavuz <[email protected]>
Date: Fri, 10 May 2024 10:43:45 +0300
Subject: [PATCH v1] Fix -Warray-bounds warning in guc_var_compare() function

There are a couple of places that guc_var_compare() function takes
'const char ***' type and then casts it to the
'const struct config_generic *' type. This triggers '-Warray-bounds'
warning. So, instead cast them to the 'const char *' type.

Author: Nazir Bilal Yavuz <[email protected]>
Reported-by: Erik Rijkers <[email protected]>
Suggested-by: Andres Freund <[email protected]>
Discussion: https://postgr.es/m/a74a1a0d-0fd2-3649-5224-4f754e8f91aa%40xs4all.nl
---
 src/backend/utils/misc/guc.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index c410ba532d2..eec97dea659 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -5721,10 +5721,10 @@ find_option(const char *name, bool create_placeholders, bool skip_errors,
 static int
 guc_var_compare(const void *a, const void *b)
 {
-	const struct config_generic *confa = *(struct config_generic *const *) a;
-	const struct config_generic *confb = *(struct config_generic *const *) b;
+	const char *confa_name = **(char **const *) a;
+	const char *confb_name = **(char **const *) b;
 
-	return guc_name_compare(confa->name, confb->name);
+	return guc_name_compare(confa_name, confb_name);
 }
 
 /*
-- 
2.43.0



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

* Re: gcc 12.1.0 warning
  2024-04-15 08:25 Re: gcc 12.1.0 warning Nazir Bilal Yavuz <[email protected]>
  2024-04-23 16:59 ` Re: gcc 12.1.0 warning Andres Freund <[email protected]>
  2024-05-10 09:13   ` Re: gcc 12.1.0 warning Nazir Bilal Yavuz <[email protected]>
@ 2024-07-15 16:41     ` Andres Freund <[email protected]>
  0 siblings, 0 replies; 51+ messages in thread

From: Andres Freund @ 2024-07-15 16:41 UTC (permalink / raw)
  To: Nazir Bilal Yavuz <[email protected]>; +Cc: Tom Lane <[email protected]>; Erik Rijkers <[email protected]>; [email protected]; Thomas Munro <[email protected]>

Hi,

On 2024-05-10 12:13:21 +0300, Nazir Bilal Yavuz wrote:
> On Tue, 23 Apr 2024 at 19:59, Andres Freund <[email protected]> wrote:
> >
> >
> > Which seems entirely legitimate. ISTM that guc_var_compare() ought to only
> > cast the pointers to the key type, i.e. char *.  And incidentally that does
> > prevent the warning.
> >
> > The reason it doesn't happen in newer versions of postgres is that we aren't
> > using guc_var_compare() in the relevant places anymore...
> 
> The fix is attached. It cleanly applies from REL_15_STABLE to
> REL_12_STABLE, fixes the warnings and the tests pass.

Thanks! I've applied it to all branches - while it's not required to avoid a
warning in newer versions, it's still not correct as it was...

Greetings,

Andres






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

end of thread, other threads:[~2024-07-15 16:41 UTC | newest]

Thread overview: 51+ 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]>
2021-01-20 22:06 [PATCH v6 3/3] Optimize hash function Heikki Linnakangas <[email protected]>
2021-01-20 22:06 [PATCH v7 3/3] Optimize hash function Heikki Linnakangas <[email protected]>
2021-01-20 22:06 [PATCH v6 3/3] Optimize hash function Heikki Linnakangas <[email protected]>
2021-01-20 22:06 [PATCH v6 3/3] Optimize hash function Heikki Linnakangas <[email protected]>
2021-01-20 22:06 [PATCH v7 3/3] Optimize hash function Heikki Linnakangas <[email protected]>
2021-01-20 22:06 [PATCH v7 3/3] Optimize hash function Heikki Linnakangas <[email protected]>
2021-01-20 22:06 [PATCH v7 3/3] Optimize hash function Heikki Linnakangas <[email protected]>
2021-01-20 22:06 [PATCH v6 3/3] Optimize hash function Heikki Linnakangas <[email protected]>
2021-01-20 22:06 [PATCH v7 3/3] Optimize hash function Heikki Linnakangas <[email protected]>
2021-01-20 22:06 [PATCH v7 3/3] Optimize hash function Heikki Linnakangas <[email protected]>
2021-01-20 22:06 [PATCH v7 3/3] Optimize hash function Heikki Linnakangas <[email protected]>
2021-01-20 22:06 [PATCH v7 3/3] Optimize hash function Heikki Linnakangas <[email protected]>
2021-01-20 22:06 [PATCH v6 3/3] Optimize hash function Heikki Linnakangas <[email protected]>
2021-01-20 22:06 [PATCH v6 3/3] Optimize hash function Heikki Linnakangas <[email protected]>
2021-01-20 22:06 [PATCH v7 3/3] Optimize hash function Heikki Linnakangas <[email protected]>
2021-01-20 22:06 [PATCH v6 3/3] Optimize hash function Heikki Linnakangas <[email protected]>
2021-01-20 22:06 [PATCH v7 3/3] Optimize hash function Heikki Linnakangas <[email protected]>
2021-01-20 22:06 [PATCH v7 3/3] Optimize hash function Heikki Linnakangas <[email protected]>
2021-01-20 22:06 [PATCH v6 3/3] Optimize hash function Heikki Linnakangas <[email protected]>
2021-01-20 22:06 [PATCH v7 3/3] Optimize hash function Heikki Linnakangas <[email protected]>
2021-01-20 22:06 [PATCH v7 3/3] Optimize hash function Heikki Linnakangas <[email protected]>
2021-01-20 22:06 [PATCH v6 3/3] Optimize hash function Heikki Linnakangas <[email protected]>
2021-01-20 22:06 [PATCH v7 3/3] Optimize hash function Heikki Linnakangas <[email protected]>
2021-01-20 22:06 [PATCH v7 3/3] Optimize hash function Heikki Linnakangas <[email protected]>
2021-01-20 22:06 [PATCH v6 3/3] Optimize hash function Heikki Linnakangas <[email protected]>
2021-01-20 22:06 [PATCH v7 3/3] Optimize hash function Heikki Linnakangas <[email protected]>
2021-01-20 22:06 [PATCH v6 3/3] Optimize hash function Heikki Linnakangas <[email protected]>
2021-01-20 22:06 [PATCH v6 3/3] Optimize hash function Heikki Linnakangas <[email protected]>
2021-01-20 22:06 [PATCH v6 3/3] Optimize hash function Heikki Linnakangas <[email protected]>
2021-01-20 22:06 [PATCH v7 3/3] Optimize hash function Heikki Linnakangas <[email protected]>
2021-01-20 22:06 [PATCH v6 3/3] Optimize hash function Heikki Linnakangas <[email protected]>
2021-01-20 22:06 [PATCH v6 3/3] Optimize hash function Heikki Linnakangas <[email protected]>
2021-01-20 22:06 [PATCH v7 3/3] Optimize hash function Heikki Linnakangas <[email protected]>
2021-01-20 22:06 [PATCH v7 3/3] Optimize hash function Heikki Linnakangas <[email protected]>
2021-01-20 22:06 [PATCH v7 3/3] Optimize hash function Heikki Linnakangas <[email protected]>
2021-01-20 22:06 [PATCH v6 3/3] Optimize hash function Heikki Linnakangas <[email protected]>
2021-01-20 22:06 [PATCH v7 3/3] Optimize hash function Heikki Linnakangas <[email protected]>
2021-01-20 22:06 [PATCH v7 3/3] Optimize hash function Heikki Linnakangas <[email protected]>
2021-01-20 22:06 [PATCH v7 3/3] Optimize hash function Heikki Linnakangas <[email protected]>
2021-01-20 22:06 [PATCH v6 3/3] Optimize hash function Heikki Linnakangas <[email protected]>
2021-01-20 22:06 [PATCH v6 3/3] Optimize hash function Heikki Linnakangas <[email protected]>
2021-01-20 22:06 [PATCH v7 3/3] Optimize hash function Heikki Linnakangas <[email protected]>
2021-01-20 22:06 [PATCH v6 3/3] Optimize hash function Heikki Linnakangas <[email protected]>
2021-01-20 22:06 [PATCH v6 3/3] Optimize hash function Heikki Linnakangas <[email protected]>
2021-01-20 22:06 [PATCH v8 3/3] Optimize hash function Heikki Linnakangas <[email protected]>
2021-10-18 10:59 [PATCH v10 3/3] Optimize hash function Heikki Linnakangas <[email protected]>
2024-04-15 08:25 Re: gcc 12.1.0 warning Nazir Bilal Yavuz <[email protected]>
2024-04-23 16:59 ` Re: gcc 12.1.0 warning Andres Freund <[email protected]>
2024-05-10 09:13   ` Re: gcc 12.1.0 warning Nazir Bilal Yavuz <[email protected]>
2024-07-15 16:41     ` Re: gcc 12.1.0 warning Andres Freund <[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