From d8cc1616b5547d49d899be7b68713272ce334708 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Wed, 17 Oct 2018 14:22:33 -0400 Subject: [PATCH] Minor additional improvements for ecpglib/prepare.c. Avoid allocating never-used entries in stmtCacheEntries[], other than the intentionally-unused zero'th entry. Tie the array size directly to the bucket count and size, rather than having undocumented dependencies between three magic constants. Fix the hash calculation to be platform-independent --- notably, it was sensitive to the signed'ness of "char" before, not to mention having an unnecessary hard-wired dependency on the existence and size of type "long long". (The lack of complaints says it's been a long time since anybody tried to build PG on a compiler without "long long", and certainly with the requirement for C99 this isn't a live bug anymore. But it's still not per project coding style.) Fix ecpg_auto_prepare's new-cache-entry path so that it increments the exec count for the new cache entry not the dummy zero'th entry. The last of those is an actual bug, though one of little consequence; the rest is mostly future-proofing and neatnik-ism. Doesn't seem necessary to back-patch. --- src/interfaces/ecpg/ecpglib/prepare.c | 33 ++++++++++++------- .../test/expected/preproc-autoprep.stderr | 10 +++--- 2 files changed, 27 insertions(+), 16 deletions(-) diff --git a/src/interfaces/ecpg/ecpglib/prepare.c b/src/interfaces/ecpg/ecpglib/prepare.c index f904d326e4..5368886f67 100644 --- a/src/interfaces/ecpg/ecpglib/prepare.c +++ b/src/interfaces/ecpg/ecpglib/prepare.c @@ -13,7 +13,17 @@ #define STMTID_SIZE 32 -#define N_STMTCACHE_ENTRIES 16384 +/* + * The statement cache contains stmtCacheNBuckets hash buckets, each + * having stmtCacheEntPerBucket entries, which we recycle as needed, + * giving up the least-executed entry in the bucket. + * stmtCacheEntries[0] is never used, so that zero can be a "not found" + * indicator. + */ +#define stmtCacheNBuckets 2039 /* should be a prime number */ +#define stmtCacheEntPerBucket 8 + +#define stmtCacheArraySize (stmtCacheNBuckets * stmtCacheEntPerBucket + 1) typedef struct { @@ -25,8 +35,6 @@ typedef struct } stmtCacheEntry; static int nextStmtID = 1; -static const int stmtCacheNBuckets = 2039; /* # buckets - a prime # */ -static const int stmtCacheEntPerBucket = 8; /* # entries/bucket */ static stmtCacheEntry *stmtCacheEntries = NULL; static bool deallocate_one(int lineno, enum COMPAT_MODE c, struct connection *con, @@ -330,7 +338,7 @@ HashStmt(const char *ecpgQuery) bucketNo, hashLeng, stmtLeng; - long long hashVal, + uint64 hashVal, rotVal; stmtLeng = strlen(ecpgQuery); @@ -341,16 +349,17 @@ HashStmt(const char *ecpgQuery) hashVal = 0; for (stmtIx = 0; stmtIx < hashLeng; ++stmtIx) { - hashVal = hashVal + (int) ecpgQuery[stmtIx]; + hashVal = hashVal + (unsigned char) ecpgQuery[stmtIx]; + /* rotate 32-bit hash value left 13 bits */ hashVal = hashVal << 13; - rotVal = (hashVal & 0x1fff00000000LL) >> 32; - hashVal = (hashVal & 0xffffffffLL) | rotVal; + rotVal = (hashVal & UINT64CONST(0x1fff00000000)) >> 32; + hashVal = (hashVal & UINT64CONST(0xffffffff)) | rotVal; } bucketNo = hashVal % stmtCacheNBuckets; - bucketNo += 1; /* don't use bucket # 0 */ - return (bucketNo * stmtCacheEntPerBucket); + /* Add 1 so that array entry 0 is never used */ + return bucketNo * stmtCacheEntPerBucket + 1; } /* @@ -451,7 +460,7 @@ AddStmtToCache(int lineno, /* line # of statement */ if (stmtCacheEntries == NULL) { stmtCacheEntries = (stmtCacheEntry *) - ecpg_alloc(sizeof(stmtCacheEntry) * N_STMTCACHE_ENTRIES, lineno); + ecpg_alloc(sizeof(stmtCacheEntry) * stmtCacheArraySize, lineno); if (stmtCacheEntries == NULL) return -1; } @@ -534,7 +543,9 @@ ecpg_auto_prepare(int lineno, const char *connection_name, const int compat, cha if (!ECPGprepare(lineno, connection_name, 0, stmtID, query)) return false; - if (AddStmtToCache(lineno, stmtID, connection_name, compat, query) < 0) + + entNo = AddStmtToCache(lineno, stmtID, connection_name, compat, query); + if (entNo < 0) return false; *name = ecpg_strdup(stmtID, lineno); diff --git a/src/interfaces/ecpg/test/expected/preproc-autoprep.stderr b/src/interfaces/ecpg/test/expected/preproc-autoprep.stderr index ea21e82ca6..bfeea59a75 100644 --- a/src/interfaces/ecpg/test/expected/preproc-autoprep.stderr +++ b/src/interfaces/ecpg/test/expected/preproc-autoprep.stderr @@ -30,7 +30,7 @@ [NO_PID]: sqlca: code: 0, state: 00000 [NO_PID]: ecpg_process_output on line 24: OK: INSERT 0 1 [NO_PID]: sqlca: code: 0, state: 00000 -[NO_PID]: ecpg_auto_prepare on line 26: statement found in cache; entry 1640 +[NO_PID]: ecpg_auto_prepare on line 26: statement found in cache; entry 1633 [NO_PID]: sqlca: code: 0, state: 00000 [NO_PID]: ecpg_execute on line 26: query: insert into T values ( 1 , $1 ); with 1 parameter(s) on connection ecpg1_regression [NO_PID]: sqlca: code: 0, state: 00000 @@ -168,7 +168,7 @@ [NO_PID]: sqlca: code: 0, state: 00000 [NO_PID]: ecpg_process_output on line 21: OK: CREATE TABLE [NO_PID]: sqlca: code: 0, state: 00000 -[NO_PID]: ecpg_auto_prepare on line 23: statement found in cache; entry 15328 +[NO_PID]: ecpg_auto_prepare on line 23: statement found in cache; entry 15321 [NO_PID]: sqlca: code: 0, state: 00000 [NO_PID]: prepare_common on line 23: name ecpg1; query: "insert into T values ( 1 , null )" [NO_PID]: sqlca: code: 0, state: 00000 @@ -178,7 +178,7 @@ [NO_PID]: sqlca: code: 0, state: 00000 [NO_PID]: ecpg_process_output on line 23: OK: INSERT 0 1 [NO_PID]: sqlca: code: 0, state: 00000 -[NO_PID]: ecpg_auto_prepare on line 24: statement found in cache; entry 1640 +[NO_PID]: ecpg_auto_prepare on line 24: statement found in cache; entry 1633 [NO_PID]: sqlca: code: 0, state: 00000 [NO_PID]: prepare_common on line 24: name ecpg2; query: "insert into T values ( 1 , $1 )" [NO_PID]: sqlca: code: 0, state: 00000 @@ -190,7 +190,7 @@ [NO_PID]: sqlca: code: 0, state: 00000 [NO_PID]: ecpg_process_output on line 24: OK: INSERT 0 1 [NO_PID]: sqlca: code: 0, state: 00000 -[NO_PID]: ecpg_auto_prepare on line 26: statement found in cache; entry 1640 +[NO_PID]: ecpg_auto_prepare on line 26: statement found in cache; entry 1633 [NO_PID]: sqlca: code: 0, state: 00000 [NO_PID]: ecpg_execute on line 26: query: insert into T values ( 1 , $1 ); with 1 parameter(s) on connection ecpg1_regression [NO_PID]: sqlca: code: 0, state: 00000 @@ -208,7 +208,7 @@ [NO_PID]: sqlca: code: 0, state: 00000 [NO_PID]: ecpg_process_output on line 28: OK: INSERT 0 1 [NO_PID]: sqlca: code: 0, state: 00000 -[NO_PID]: ecpg_auto_prepare on line 30: statement found in cache; entry 13056 +[NO_PID]: ecpg_auto_prepare on line 30: statement found in cache; entry 13049 [NO_PID]: sqlca: code: 0, state: 00000 [NO_PID]: prepare_common on line 30: name ecpg3; query: "select Item2 from T order by Item2 nulls last" [NO_PID]: sqlca: code: 0, state: 00000 -- 2.40.0