Improve hash_create's API for selecting simple-binary-key hash functions.

Previously, if you wanted anything besides C-string hash keys, you had to
specify a custom hashing function to hash_create().  Nearly all such
callers were specifying tag_hash or oid_hash; which is tedious, and rather
error-prone, since a caller could easily miss the opportunity to optimize
by using hash_uint32 when appropriate.  Replace this with a design whereby
callers using simple binary-data keys just specify HASH_BLOBS and don't
need to mess with specific support functions.  hash_create() itself will
take care of optimizing when the key size is four bytes.

This nets out saving a few hundred bytes of code space, and offers
a measurable performance improvement in tidbitmap.c (which was not
exploiting the opportunity to use hash_uint32 for its 4-byte keys).
There might be some wins elsewhere too, I didn't analyze closely.

In future we could look into offering a similar optimized hashing function
for 8-byte keys.  Under this design that could be done in a centralized
and machine-independent fashion, whereas getting it right for keys of
platform-dependent sizes would've been notationally painful before.

For the moment, the old way still works fine, so as not to break source
code compatibility for loadable modules.  Eventually we might want to
remove tag_hash and friends from the exported API altogether, since there's
no real need for them to be explicitly referenced from outside dynahash.c.

Teodor Sigaev and Tom Lane

14 files changed, 55 insertions, 41 deletions

diff --git a/src/backend/utils/adt/array_typanalyze.c b/src/backend/utils/adt/array_typanalyze.c
index 4d7e9c311fb..3653888f399 100644
--- a/src/backend/utils/adt/array_typanalyze.c
+++ b/src/backend/utils/adt/array_typanalyze.c
@@ -290,12 +290,11 @@ compute_array_stats(VacAttrStats *stats, AnalyzeAttrFetchFunc fetchfunc,
 	MemSet(&count_hash_ctl, 0, sizeof(count_hash_ctl));
 	count_hash_ctl.keysize = sizeof(int);
 	count_hash_ctl.entrysize = sizeof(DECountItem);
-	count_hash_ctl.hash = tag_hash;
 	count_hash_ctl.hcxt = CurrentMemoryContext;
 	count_tab = hash_create("Array distinct element count table",
 							64,
 							&count_hash_ctl,
-							HASH_ELEM | HASH_FUNCTION | HASH_CONTEXT);
+							HASH_ELEM | HASH_BLOBS | HASH_CONTEXT);
 
 	/* Initialize counters. */
 	b_current = 1;
diff --git a/src/backend/utils/adt/pg_locale.c b/src/backend/utils/adt/pg_locale.c
index 94bb5a47bb7..1b6874303d5 100644
--- a/src/backend/utils/adt/pg_locale.c
+++ b/src/backend/utils/adt/pg_locale.c
@@ -875,9 +875,8 @@ lookup_collation_cache(Oid collation, bool set_flags)
 		memset(&ctl, 0, sizeof(ctl));
 		ctl.keysize = sizeof(Oid);
 		ctl.entrysize = sizeof(collation_cache_entry);
-		ctl.hash = oid_hash;
 		collation_cache = hash_create("Collation cache", 100, &ctl,
-									  HASH_ELEM | HASH_FUNCTION);
+									  HASH_ELEM | HASH_BLOBS);
 	}
 
 	cache_entry = hash_search(collation_cache, &collation, HASH_ENTER, &found);
diff --git a/src/backend/utils/adt/ri_triggers.c b/src/backend/utils/adt/ri_triggers.c
index 2f0230384f4..5c75390ce49 100644
--- a/src/backend/utils/adt/ri_triggers.c
+++ b/src/backend/utils/adt/ri_triggers.c
@@ -3326,10 +3326,9 @@ ri_InitHashTables(void)
 	memset(&ctl, 0, sizeof(ctl));
 	ctl.keysize = sizeof(Oid);
 	ctl.entrysize = sizeof(RI_ConstraintInfo);
-	ctl.hash = oid_hash;
 	ri_constraint_cache = hash_create("RI constraint cache",
 									  RI_INIT_CONSTRAINTHASHSIZE,
-									  &ctl, HASH_ELEM | HASH_FUNCTION);
+									  &ctl, HASH_ELEM | HASH_BLOBS);
 
 	/* Arrange to flush cache on pg_constraint changes */
 	CacheRegisterSyscacheCallback(CONSTROID,
@@ -3339,18 +3338,16 @@ ri_InitHashTables(void)
 	memset(&ctl, 0, sizeof(ctl));
 	ctl.keysize = sizeof(RI_QueryKey);
 	ctl.entrysize = sizeof(RI_QueryHashEntry);
-	ctl.hash = tag_hash;
 	ri_query_cache = hash_create("RI query cache",
 								 RI_INIT_QUERYHASHSIZE,
-								 &ctl, HASH_ELEM | HASH_FUNCTION);
+								 &ctl, HASH_ELEM | HASH_BLOBS);
 
 	memset(&ctl, 0, sizeof(ctl));
 	ctl.keysize = sizeof(RI_CompareKey);
 	ctl.entrysize = sizeof(RI_CompareHashEntry);
-	ctl.hash = tag_hash;
 	ri_compare_cache = hash_create("RI compare cache",
 								   RI_INIT_QUERYHASHSIZE,
-								   &ctl, HASH_ELEM | HASH_FUNCTION);
+								   &ctl, HASH_ELEM | HASH_BLOBS);
 }
 
 
diff --git a/src/backend/utils/cache/attoptcache.c b/src/backend/utils/cache/attoptcache.c
index 5fcf0dd7c75..75e7ae60cac 100644
--- a/src/backend/utils/cache/attoptcache.c
+++ b/src/backend/utils/cache/attoptcache.c
@@ -82,10 +82,9 @@ InitializeAttoptCache(void)
 	MemSet(&ctl, 0, sizeof(ctl));
 	ctl.keysize = sizeof(AttoptCacheKey);
 	ctl.entrysize = sizeof(AttoptCacheEntry);
-	ctl.hash = tag_hash;
 	AttoptCacheHash =
 		hash_create("Attopt cache", 256, &ctl,
-					HASH_ELEM | HASH_FUNCTION);
+					HASH_ELEM | HASH_BLOBS);
 
 	/* Make sure we've initialized CacheMemoryContext. */
 	if (!CacheMemoryContext)
diff --git a/src/backend/utils/cache/evtcache.c b/src/backend/utils/cache/evtcache.c
index b9d442cdfc9..01051b08a0d 100644
--- a/src/backend/utils/cache/evtcache.c
+++ b/src/backend/utils/cache/evtcache.c
@@ -123,10 +123,9 @@ BuildEventTriggerCache(void)
 	MemSet(&ctl, 0, sizeof(ctl));
 	ctl.keysize = sizeof(EventTriggerEvent);
 	ctl.entrysize = sizeof(EventTriggerCacheEntry);
-	ctl.hash = tag_hash;
 	ctl.hcxt = EventTriggerCacheContext;
 	cache = hash_create("Event Trigger Cache", 32, &ctl,
-						HASH_ELEM | HASH_FUNCTION | HASH_CONTEXT);
+						HASH_ELEM | HASH_BLOBS | HASH_CONTEXT);
 
 	/*
 	 * Prepare to scan pg_event_trigger in name order.
diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c
index 79244e56865..c2e574da482 100644
--- a/src/backend/utils/cache/relcache.c
+++ b/src/backend/utils/cache/relcache.c
@@ -1409,9 +1409,8 @@ LookupOpclassInfo(Oid operatorClassOid,
 		MemSet(&ctl, 0, sizeof(ctl));
 		ctl.keysize = sizeof(Oid);
 		ctl.entrysize = sizeof(OpClassCacheEnt);
-		ctl.hash = oid_hash;
 		OpClassCache = hash_create("Operator class cache", 64,
-								   &ctl, HASH_ELEM | HASH_FUNCTION);
+								   &ctl, HASH_ELEM | HASH_BLOBS);
 
 		/* Also make sure CacheMemoryContext exists */
 		if (!CacheMemoryContext)
@@ -3140,9 +3139,8 @@ RelationCacheInitialize(void)
 	MemSet(&ctl, 0, sizeof(ctl));
 	ctl.keysize = sizeof(Oid);
 	ctl.entrysize = sizeof(RelIdCacheEnt);
-	ctl.hash = oid_hash;
 	RelationIdCache = hash_create("Relcache by OID", INITRELCACHESIZE,
-								  &ctl, HASH_ELEM | HASH_FUNCTION);
+								  &ctl, HASH_ELEM | HASH_BLOBS);
 
 	/*
 	 * relation mapper needs to be initialized too
diff --git a/src/backend/utils/cache/relfilenodemap.c b/src/backend/utils/cache/relfilenodemap.c
index 1e8429c64c3..87813fa52cb 100644
--- a/src/backend/utils/cache/relfilenodemap.c
+++ b/src/backend/utils/cache/relfilenodemap.c
@@ -115,7 +115,6 @@ InitializeRelfilenodeMap(void)
 	MemSet(&ctl, 0, sizeof(ctl));
 	ctl.keysize = sizeof(RelfilenodeMapKey);
 	ctl.entrysize = sizeof(RelfilenodeMapEntry);
-	ctl.hash = tag_hash;
 	ctl.hcxt = CacheMemoryContext;
 
 	/*
@@ -125,7 +124,7 @@ InitializeRelfilenodeMap(void)
 	 */
 	RelfilenodeMapHash =
 		hash_create("RelfilenodeMap cache", 1024, &ctl,
-					HASH_ELEM | HASH_FUNCTION | HASH_CONTEXT);
+					HASH_ELEM | HASH_BLOBS | HASH_CONTEXT);
 
 	/* Watch for invalidation events. */
 	CacheRegisterRelcacheCallback(RelfilenodeMapInvalidateCallback,
diff --git a/src/backend/utils/cache/spccache.c b/src/backend/utils/cache/spccache.c
index 24e8679e254..6cbafaf6919 100644
--- a/src/backend/utils/cache/spccache.c
+++ b/src/backend/utils/cache/spccache.c
@@ -81,10 +81,9 @@ InitializeTableSpaceCache(void)
 	MemSet(&ctl, 0, sizeof(ctl));
 	ctl.keysize = sizeof(Oid);
 	ctl.entrysize = sizeof(TableSpaceCacheEntry);
-	ctl.hash = oid_hash;
 	TableSpaceCacheHash =
 		hash_create("TableSpace cache", 16, &ctl,
-					HASH_ELEM | HASH_FUNCTION);
+					HASH_ELEM | HASH_BLOBS);
 
 	/* Make sure we've initialized CacheMemoryContext. */
 	if (!CacheMemoryContext)
diff --git a/src/backend/utils/cache/ts_cache.c b/src/backend/utils/cache/ts_cache.c
index 5ff1461f7d8..46da30357bb 100644
--- a/src/backend/utils/cache/ts_cache.c
+++ b/src/backend/utils/cache/ts_cache.c
@@ -120,9 +120,8 @@ lookup_ts_parser_cache(Oid prsId)
 		MemSet(&ctl, 0, sizeof(ctl));
 		ctl.keysize = sizeof(Oid);
 		ctl.entrysize = sizeof(TSParserCacheEntry);
-		ctl.hash = oid_hash;
 		TSParserCacheHash = hash_create("Tsearch parser cache", 4,
-										&ctl, HASH_ELEM | HASH_FUNCTION);
+										&ctl, HASH_ELEM | HASH_BLOBS);
 		/* Flush cache on pg_ts_parser changes */
 		CacheRegisterSyscacheCallback(TSPARSEROID, InvalidateTSCacheCallBack,
 									  PointerGetDatum(TSParserCacheHash));
@@ -219,9 +218,8 @@ lookup_ts_dictionary_cache(Oid dictId)
 		MemSet(&ctl, 0, sizeof(ctl));
 		ctl.keysize = sizeof(Oid);
 		ctl.entrysize = sizeof(TSDictionaryCacheEntry);
-		ctl.hash = oid_hash;
 		TSDictionaryCacheHash = hash_create("Tsearch dictionary cache", 8,
-											&ctl, HASH_ELEM | HASH_FUNCTION);
+											&ctl, HASH_ELEM | HASH_BLOBS);
 		/* Flush cache on pg_ts_dict and pg_ts_template changes */
 		CacheRegisterSyscacheCallback(TSDICTOID, InvalidateTSCacheCallBack,
 									  PointerGetDatum(TSDictionaryCacheHash));
@@ -368,9 +366,8 @@ init_ts_config_cache(void)
 	MemSet(&ctl, 0, sizeof(ctl));
 	ctl.keysize = sizeof(Oid);
 	ctl.entrysize = sizeof(TSConfigCacheEntry);
-	ctl.hash = oid_hash;
 	TSConfigCacheHash = hash_create("Tsearch configuration cache", 16,
-									&ctl, HASH_ELEM | HASH_FUNCTION);
+									&ctl, HASH_ELEM | HASH_BLOBS);
 	/* Flush cache on pg_ts_config and pg_ts_config_map changes */
 	CacheRegisterSyscacheCallback(TSCONFIGOID, InvalidateTSCacheCallBack,
 								  PointerGetDatum(TSConfigCacheHash));
diff --git a/src/backend/utils/cache/typcache.c b/src/backend/utils/cache/typcache.c
index 29aec3cf743..08e17b92d22 100644
--- a/src/backend/utils/cache/typcache.c
+++ b/src/backend/utils/cache/typcache.c
@@ -166,9 +166,8 @@ lookup_type_cache(Oid type_id, int flags)
 		MemSet(&ctl, 0, sizeof(ctl));
 		ctl.keysize = sizeof(Oid);
 		ctl.entrysize = sizeof(TypeCacheEntry);
-		ctl.hash = oid_hash;
 		TypeCacheHash = hash_create("Type information cache", 64,
-									&ctl, HASH_ELEM | HASH_FUNCTION);
+									&ctl, HASH_ELEM | HASH_BLOBS);
 
 		/* Also set up callbacks for SI invalidations */
 		CacheRegisterRelcacheCallback(TypeCacheRelCallback, (Datum) 0);
@@ -846,9 +845,8 @@ assign_record_type_typmod(TupleDesc tupDesc)
 		MemSet(&ctl, 0, sizeof(ctl));
 		ctl.keysize = REC_HASH_KEYS * sizeof(Oid);
 		ctl.entrysize = sizeof(RecordCacheEntry);
-		ctl.hash = tag_hash;
 		RecordCacheHash = hash_create("Record information cache", 64,
-									  &ctl, HASH_ELEM | HASH_FUNCTION);
+									  &ctl, HASH_ELEM | HASH_BLOBS);
 
 		/* Also make sure CacheMemoryContext exists */
 		if (!CacheMemoryContext)
diff --git a/src/backend/utils/fmgr/fmgr.c b/src/backend/utils/fmgr/fmgr.c
index a95be056dae..e81c8553152 100644
--- a/src/backend/utils/fmgr/fmgr.c
+++ b/src/backend/utils/fmgr/fmgr.c
@@ -540,11 +540,10 @@ record_C_func(HeapTuple procedureTuple,
 		MemSet(&hash_ctl, 0, sizeof(hash_ctl));
 		hash_ctl.keysize = sizeof(Oid);
 		hash_ctl.entrysize = sizeof(CFuncHashTabEntry);
-		hash_ctl.hash = oid_hash;
 		CFuncHash = hash_create("CFuncHash",
 								100,
 								&hash_ctl,
-								HASH_ELEM | HASH_FUNCTION);
+								HASH_ELEM | HASH_BLOBS);
 	}
 
 	entry = (CFuncHashTabEntry *)
diff --git a/src/backend/utils/hash/dynahash.c b/src/backend/utils/hash/dynahash.c
index 2b99e4b5218..5f6d6cbbde5 100644
--- a/src/backend/utils/hash/dynahash.c
+++ b/src/backend/utils/hash/dynahash.c
@@ -26,6 +26,20 @@
  * in local memory, we typically use palloc() which will throw error on
  * failure.  The code in this file has to cope with both cases.
  *
+ * dynahash.c provides support for these types of lookup keys:
+ *
+ * 1. Null-terminated C strings (truncated if necessary to fit in keysize),
+ * compared as though by strcmp().  This is the default behavior.
+ *
+ * 2. Arbitrary binary data of size keysize, compared as though by memcmp().
+ * (Caller must ensure there are no undefined padding bits in the keys!)
+ * This is selected by specifying HASH_BLOBS flag to hash_create.
+ *
+ * 3. More complex key behavior can be selected by specifying user-supplied
+ * hashing, comparison, and/or key-copying functions.  At least a hashing
+ * function must be supplied; comparison defaults to memcmp() and key copying
+ * to memcpy() when a user-defined hashing function is selected.
+ *
  * Portions Copyright (c) 1996-2014, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
@@ -305,15 +319,32 @@ hash_create(const char *tabname, long nelem, HASHCTL *info, int flags)
 	hashp->tabname = (char *) (hashp + 1);
 	strcpy(hashp->tabname, tabname);
 
+	/*
+	 * Select the appropriate hash function (see comments at head of file).
+	 */
 	if (flags & HASH_FUNCTION)
 		hashp->hash = info->hash;
+	else if (flags & HASH_BLOBS)
+	{
+		/* We can optimize hashing for common key sizes */
+		Assert(flags & HASH_ELEM);
+		if (info->keysize == sizeof(uint32))
+			hashp->hash = uint32_hash;
+		else
+			hashp->hash = tag_hash;
+	}
 	else
 		hashp->hash = string_hash;		/* default hash function */
 
 	/*
 	 * If you don't specify a match function, it defaults to string_compare if
 	 * you used string_hash (either explicitly or by default) and to memcmp
-	 * otherwise.  (Prior to PostgreSQL 7.4, memcmp was always used.)
+	 * otherwise.
+	 *
+	 * Note: explicitly specifying string_hash is deprecated, because this
+	 * might not work for callers in loadable modules on some platforms due to
+	 * referencing a trampoline instead of the string_hash function proper.
+	 * Just let it default, eh?
 	 */
 	if (flags & HASH_COMPARE)
 		hashp->match = info->match;
@@ -332,6 +363,7 @@ hash_create(const char *tabname, long nelem, HASHCTL *info, int flags)
 	else
 		hashp->keycopy = memcpy;
 
+	/* And select the entry allocation function, too. */
 	if (flags & HASH_ALLOC)
 		hashp->alloc = info->alloc;
 	else
diff --git a/src/backend/utils/hash/hashfn.c b/src/backend/utils/hash/hashfn.c
index a12f98f2f25..adb0bd9564c 100644
--- a/src/backend/utils/hash/hashfn.c
+++ b/src/backend/utils/hash/hashfn.c
@@ -55,15 +55,15 @@ tag_hash(const void *key, Size keysize)
 }
 
 /*
- * oid_hash: hash function for keys that are OIDs
+ * uint32_hash: hash function for keys that are uint32 or int32
  *
  * (tag_hash works for this case too, but is slower)
  */
 uint32
-oid_hash(const void *key, Size keysize)
+uint32_hash(const void *key, Size keysize)
 {
-	Assert(keysize == sizeof(Oid));
-	return DatumGetUInt32(hash_uint32((uint32) *((const Oid *) key)));
+	Assert(keysize == sizeof(uint32));
+	return DatumGetUInt32(hash_uint32(*((const uint32 *) key)));
 }
 
 /*
diff --git a/src/backend/utils/time/combocid.c b/src/backend/utils/time/combocid.c
index a70c7542c92..ea7a905db4a 100644
--- a/src/backend/utils/time/combocid.c
+++ b/src/backend/utils/time/combocid.c
@@ -218,13 +218,12 @@ GetComboCommandId(CommandId cmin, CommandId cmax)
 		memset(&hash_ctl, 0, sizeof(hash_ctl));
 		hash_ctl.keysize = sizeof(ComboCidKeyData);
 		hash_ctl.entrysize = sizeof(ComboCidEntryData);
-		hash_ctl.hash = tag_hash;
 		hash_ctl.hcxt = TopTransactionContext;
 
 		comboHash = hash_create("Combo CIDs",
 								CCID_HASH_SIZE,
 								&hash_ctl,
-								HASH_ELEM | HASH_FUNCTION | HASH_CONTEXT);
+								HASH_ELEM | HASH_BLOBS | HASH_CONTEXT);
 
 		comboCids = (ComboCidKeyData *)
 			MemoryContextAlloc(TopTransactionContext,