Skip to content

Commit 4a14f13

Browse files
tglsfdctglsfdc
committed
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
1 parent ba94518 commit 4a14f13

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

42 files changed

+127
-155
lines changed

‎contrib/pg_trgm/trgm_regexp.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -915,11 +915,10 @@ transformGraph(TrgmNFA *trgmNFA)
915915
hashCtl.keysize = sizeof(TrgmStateKey);
916916
hashCtl.entrysize = sizeof(TrgmState);
917917
hashCtl.hcxt = CurrentMemoryContext;
918-
hashCtl.hash = tag_hash;
919918
trgmNFA->states = hash_create("Trigram NFA",
920919
1024,
921920
&hashCtl,
922-
HASH_ELEM | HASH_CONTEXT | HASH_FUNCTION);
921+
HASH_ELEM | HASH_BLOBS | HASH_CONTEXT);
923922

924923
/* Create initial state: ambiguous prefix, NFA's initial state */
925924
MemSet(&initkey, 0, sizeof(initkey));

‎contrib/postgres_fdw/connection.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -109,12 +109,11 @@ GetConnection(ForeignServer *server, UserMapping *user,
109109
MemSet(&ctl, 0, sizeof(ctl));
110110
ctl.keysize = sizeof(ConnCacheKey);
111111
ctl.entrysize = sizeof(ConnCacheEntry);
112-
ctl.hash = tag_hash;
113112
/* allocate ConnectionHash in the cache context */
114113
ctl.hcxt = CacheMemoryContext;
115114
ConnectionHash = hash_create("postgres_fdw connections", 8,
116115
&ctl,
117-
HASH_ELEM | HASH_FUNCTION | HASH_CONTEXT);
116+
HASH_ELEM | HASH_BLOBS | HASH_CONTEXT);
118117

119118
/*
120119
* Register some callback functions that manage connection cleanup.

‎src/backend/access/gist/gistbuild.c

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1142,12 +1142,10 @@ gistInitParentMap(GISTBuildState *buildstate)
11421142
hashCtl.keysize = sizeof(BlockNumber);
11431143
hashCtl.entrysize = sizeof(ParentMapEntry);
11441144
hashCtl.hcxt = CurrentMemoryContext;
1145-
hashCtl.hash = oid_hash;
11461145
buildstate->parentMap = hash_create("gistbuild parent map",
11471146
1024,
11481147
&hashCtl,
1149-
HASH_ELEM | HASH_CONTEXT
1150-
| HASH_FUNCTION);
1148+
HASH_ELEM | HASH_BLOBS | HASH_CONTEXT);
11511149
}
11521150

11531151
static void

‎src/backend/access/gist/gistbuildbuffers.c

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -76,16 +76,14 @@ gistInitBuildBuffers(int pagesPerBuffer, int levelStep, int maxLevel)
7676
* nodeBuffersTab hash is association between index blocks and it's
7777
* buffers.
7878
*/
79+
memset(&hashCtl, 0, sizeof(hashCtl));
7980
hashCtl.keysize = sizeof(BlockNumber);
8081
hashCtl.entrysize = sizeof(GISTNodeBuffer);
8182
hashCtl.hcxt = CurrentMemoryContext;
82-
hashCtl.hash = tag_hash;
83-
hashCtl.match = memcmp;
8483
gfbb->nodeBuffersTab = hash_create("gistbuildbuffers",
8584
1024,
8685
&hashCtl,
87-
HASH_ELEM | HASH_CONTEXT
88-
| HASH_FUNCTION | HASH_COMPARE);
86+
HASH_ELEM | HASH_BLOBS | HASH_CONTEXT);
8987

9088
gfbb->bufferEmptyingQueue = NIL;
9189

‎src/backend/access/heap/rewriteheap.c

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -283,21 +283,20 @@ begin_heap_rewrite(Relation old_heap, Relation new_heap, TransactionId oldest_xm
283283
hash_ctl.keysize = sizeof(TidHashKey);
284284
hash_ctl.entrysize = sizeof(UnresolvedTupData);
285285
hash_ctl.hcxt = state->rs_cxt;
286-
hash_ctl.hash = tag_hash;
287286

288287
state->rs_unresolved_tups =
289288
hash_create("Rewrite / Unresolved ctids",
290289
128, /* arbitrary initial size */
291290
&hash_ctl,
292-
HASH_ELEM | HASH_FUNCTION | HASH_CONTEXT);
291+
HASH_ELEM | HASH_BLOBS | HASH_CONTEXT);
293292

294293
hash_ctl.entrysize = sizeof(OldToNewMappingData);
295294

296295
state->rs_old_new_tid_map =
297296
hash_create("Rewrite / Old to new tid map",
298297
128, /* arbitrary initial size */
299298
&hash_ctl,
300-
HASH_ELEM | HASH_FUNCTION | HASH_CONTEXT);
299+
HASH_ELEM | HASH_BLOBS | HASH_CONTEXT);
301300

302301
MemoryContextSwitchTo(old_cxt);
303302

@@ -834,13 +833,12 @@ logical_begin_heap_rewrite(RewriteState state)
834833
hash_ctl.keysize = sizeof(TransactionId);
835834
hash_ctl.entrysize = sizeof(RewriteMappingFile);
836835
hash_ctl.hcxt = state->rs_cxt;
837-
hash_ctl.hash = tag_hash;
838836

839837
state->rs_logical_mappings =
840838
hash_create("Logical rewrite mapping",
841839
128, /* arbitrary initial size */
842840
&hash_ctl,
843-
HASH_ELEM | HASH_FUNCTION | HASH_CONTEXT);
841+
HASH_ELEM | HASH_BLOBS | HASH_CONTEXT);
844842
}
845843

846844
/*

‎src/backend/access/transam/xlogutils.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -107,12 +107,11 @@ log_invalid_page(RelFileNode node, ForkNumber forkno, BlockNumber blkno,
107107
memset(&ctl, 0, sizeof(ctl));
108108
ctl.keysize = sizeof(xl_invalid_page_key);
109109
ctl.entrysize = sizeof(xl_invalid_page);
110-
ctl.hash = tag_hash;
111110

112111
invalid_page_tab = hash_create("XLOG invalid-page table",
113112
100,
114113
&ctl,
115-
HASH_ELEM | HASH_FUNCTION);
114+
HASH_ELEM | HASH_BLOBS);
116115
}
117116

118117
/* we currently assume xl_invalid_page_key contains no padding */

‎src/backend/commands/sequence.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -986,10 +986,9 @@ create_seq_hashtable(void)
986986
memset(&ctl, 0, sizeof(ctl));
987987
ctl.keysize = sizeof(Oid);
988988
ctl.entrysize = sizeof(SeqTableData);
989-
ctl.hash = oid_hash;
990989

991990
seqhashtab = hash_create("Sequence values", 16, &ctl,
992-
HASH_ELEM | HASH_FUNCTION);
991+
HASH_ELEM | HASH_BLOBS);
993992
}
994993

995994
/*

‎src/backend/nodes/tidbitmap.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -221,12 +221,11 @@ tbm_create_pagetable(TIDBitmap *tbm)
221221
MemSet(&hash_ctl, 0, sizeof(hash_ctl));
222222
hash_ctl.keysize = sizeof(BlockNumber);
223223
hash_ctl.entrysize = sizeof(PagetableEntry);
224-
hash_ctl.hash = tag_hash;
225224
hash_ctl.hcxt = tbm->mcxt;
226225
tbm->pagetable = hash_create("TIDBitmap",
227226
128, /* start small and extend */
228227
&hash_ctl,
229-
HASH_ELEM | HASH_FUNCTION | HASH_CONTEXT);
228+
HASH_ELEM | HASH_BLOBS | HASH_CONTEXT);
230229

231230
/* If entry1 is valid, push it into the hashtable */
232231
if (tbm->status == TBM_ONE_PAGE)

‎src/backend/optimizer/util/predtest.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1711,9 +1711,8 @@ lookup_proof_cache(Oid pred_op, Oid clause_op, bool refute_it)
17111711
MemSet(&ctl, 0, sizeof(ctl));
17121712
ctl.keysize = sizeof(OprProofCacheKey);
17131713
ctl.entrysize = sizeof(OprProofCacheEntry);
1714-
ctl.hash = tag_hash;
17151714
OprProofCacheHash = hash_create("Btree proof lookup cache", 256,
1716-
&ctl, HASH_ELEM | HASH_FUNCTION);
1715+
&ctl, HASH_ELEM | HASH_BLOBS);
17171716

17181717
/* Arrange to flush cache on pg_amop changes */
17191718
CacheRegisterSyscacheCallback(AMOPOPID,

‎src/backend/parser/parse_oper.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1059,9 +1059,8 @@ find_oper_cache_entry(OprCacheKey *key)
10591059
MemSet(&ctl, 0, sizeof(ctl));
10601060
ctl.keysize = sizeof(OprCacheKey);
10611061
ctl.entrysize = sizeof(OprCacheEntry);
1062-
ctl.hash = tag_hash;
10631062
OprCacheHash = hash_create("Operator lookup cache", 256,
1064-
&ctl, HASH_ELEM | HASH_FUNCTION);
1063+
&ctl, HASH_ELEM | HASH_BLOBS);
10651064

10661065
/* Arrange to flush cache on pg_operator and pg_cast changes */
10671066
CacheRegisterSyscacheCallback(OPERNAMENSP,

0 commit comments

Comments
 (0)