Replace the data structure used for keyword lookup.

Previously, ScanKeywordLookup was passed an array of string pointers. This had some performance deficiencies: the strings themselves might be scattered all over the place depending on the compiler (and some quick checking shows that at least with gcc-on-Linux, they indeed weren't reliably close together). That led to very cache-unfriendly behavior as the binary search touched strings in many different pages. Also, depending on the platform, the string pointers might need to be adjusted at program start, so that they couldn't be simple constant data. And the ScanKeyword struct had been designed with an eye to 32-bit machines originally; on 64-bit it requires 16 bytes per keyword, making it even more cache-unfriendly. Redesign so that the keyword strings themselves are allocated consecutively (as part of one big char-string constant), thereby eliminating the touch-lots-of-unrelated-pages syndrome. And get rid of the ScanKeyword array in favor of three separate arrays: uint16 offsets into the keyword array, uint16 token codes, and uint8 keyword categories. That reduces the overhead per keyword to 5 bytes instead of 16 (even less in programs that only need one of the token codes and categories); moreover, the binary search only touches the offsets array, further reducing its cache footprint. This also lets us put the token codes somewhere else than the keyword strings are, which avoids some unpleasant build dependencies. While we're at it, wrap the data used by ScanKeywordLookup into a struct that can be treated as an opaque type by most callers. That doesn't change things much right now, but it will make it less painful to switch to a hash-based lookup method, as is being discussed in the mailing list thread. Most of the change here is associated with adding a generator script that can build the new data structure from the same list-of-PG_KEYWORD header representation we used before. The PG_KEYWORD lists that plpgsql and ecpg used to embed in their scanner .c files have to be moved into headers, and the Makefiles have to be taught to invoke the generator script. This work is also necessary if we're to consider hash-based lookup, since the generator script is what would be responsible for constructing a hash table. Aside from saving a few kilobytes in each program that includes the keyword table, this seems to speed up raw parsing (flex+bison) by a few percent. So it's worth doing even as it stands, though we think we can gain even more with a follow-on patch to switch to hash-based lookup. John Naylor, with further hacking by me Discussion: https://postgr.es/m/CAJVSVGXdFVU2sgym89XPL=Lv1zOS5=EHHQ8XWNzFL=mTXkKMLw@mail.gmail.com
2025-08-27 07:42:10 +03:00 · 2019-01-06 17:02:57 -05:00
parent c5c7fa261f
commit afb0d0712f
32 changed files with 845 additions and 441 deletions
--- a/src/interfaces/ecpg/preproc/.gitignore
+++ b/src/interfaces/ecpg/preproc/.gitignore
@@ -2,5 +2,7 @@
 /preproc.c
 /preproc.h
 /pgc.c
+/c_kwlist_d.h
+/ecpg_kwlist_d.h
 /typename.c
 /ecpg
--- a/src/interfaces/ecpg/preproc/Makefile
+++ b/src/interfaces/ecpg/preproc/Makefile
@@ -28,6 +28,8 @@ OBJS=	preproc.o pgc.o type.o ecpg.o output.o parser.o \
 	keywords.o c_keywords.o ecpg_keywords.o typename.o descriptor.o variable.o \
 	$(WIN32RES)

+GEN_KEYWORDLIST = $(top_srcdir)/src/tools/gen_keywordlist.pl
+
 # Suppress parallel build to avoid a bug in GNU make 3.82
 # (see comments in ../Makefile)
 ifeq ($(MAKE_VERSION),3.82)
@@ -53,9 +55,20 @@ preproc.y: ../../../backend/parser/gram.y parse.pl ecpg.addons ecpg.header ecpg.
 	$(PERL) $(srcdir)/parse.pl $(srcdir) < $< > $@
 	$(PERL) $(srcdir)/check_rules.pl $(srcdir) $<

-ecpg_keywords.o c_keywords.o keywords.o preproc.o pgc.o parser.o: preproc.h
+# generate keyword headers
+c_kwlist_d.h: c_kwlist.h $(GEN_KEYWORDLIST)
+	$(PERL) $(GEN_KEYWORDLIST) --varname ScanCKeywords $<

-distprep: preproc.y preproc.c preproc.h pgc.c
+ecpg_kwlist_d.h: ecpg_kwlist.h $(GEN_KEYWORDLIST)
+	$(PERL) $(GEN_KEYWORDLIST) --varname ScanECPGKeywords $<
+
+# Force these dependencies to be known even without dependency info built:
+ecpg_keywords.o c_keywords.o keywords.o preproc.o pgc.o parser.o: preproc.h
+ecpg_keywords.o: ecpg_kwlist_d.h
+c_keywords.o: c_kwlist_d.h
+keywords.o: $(top_srcdir)/src/include/parser/kwlist.h
+
+distprep: preproc.y preproc.c preproc.h pgc.c c_kwlist_d.h ecpg_kwlist_d.h

 install: all installdirs
 	$(INSTALL_PROGRAM) ecpg$(X) '$(DESTDIR)$(bindir)'
@@ -66,12 +79,11 @@ installdirs:
 uninstall:
 	rm -f '$(DESTDIR)$(bindir)/ecpg$(X)'

+# preproc.y, preproc.c, preproc.h, pgc.c, c_kwlist_d.h, and ecpg_kwlist_d.h
+# are in the distribution tarball, so they are not cleaned here.
 clean distclean:
 	rm -f *.o ecpg$(X)
 	rm -f typename.c

-# `make distclean' must not remove preproc.y, preproc.c, preproc.h, or pgc.c
-# since we want to ship those files in the distribution for people with
-# inadequate tools.  Instead, `make maintainer-clean' will remove them.
 maintainer-clean: distclean
-	rm -f preproc.y preproc.c preproc.h pgc.c
+	rm -f preproc.y preproc.c preproc.h pgc.c c_kwlist_d.h ecpg_kwlist_d.h
--- a/src/interfaces/ecpg/preproc/c_keywords.c
+++ b/src/interfaces/ecpg/preproc/c_keywords.c
@@ -14,72 +14,57 @@
 #include "preproc_extern.h"
 #include "preproc.h"

-/*
- * List of (keyword-name, keyword-token-value) pairs.
- *
- * !!WARNING!!: This list must be sorted, because binary
- *		 search is used to locate entries.
- */
-static const ScanKeyword ScanCKeywords[] = {
-	/* name, value, category */
+/* ScanKeywordList lookup data for C keywords */
+#include "c_kwlist_d.h"

-	/*
-	 * category is not needed in ecpg, it is only here so we can share the
-	 * data structure with the backend
-	 */
-	{"VARCHAR", VARCHAR, 0},
-	{"auto", S_AUTO, 0},
-	{"bool", SQL_BOOL, 0},
-	{"char", CHAR_P, 0},
-	{"const", S_CONST, 0},
-	{"enum", ENUM_P, 0},
-	{"extern", S_EXTERN, 0},
-	{"float", FLOAT_P, 0},
-	{"hour", HOUR_P, 0},
-	{"int", INT_P, 0},
-	{"long", SQL_LONG, 0},
-	{"minute", MINUTE_P, 0},
-	{"month", MONTH_P, 0},
-	{"register", S_REGISTER, 0},
-	{"second", SECOND_P, 0},
-	{"short", SQL_SHORT, 0},
-	{"signed", SQL_SIGNED, 0},
-	{"static", S_STATIC, 0},
-	{"struct", SQL_STRUCT, 0},
-	{"to", TO, 0},
-	{"typedef", S_TYPEDEF, 0},
-	{"union", UNION, 0},
-	{"unsigned", SQL_UNSIGNED, 0},
-	{"varchar", VARCHAR, 0},
-	{"volatile", S_VOLATILE, 0},
-	{"year", YEAR_P, 0},
+/* Token codes for C keywords */
+#define PG_KEYWORD(kwname, value) value,
+
+static const uint16 ScanCKeywordTokens[] = {
+#include "c_kwlist.h"
 };

+#undef PG_KEYWORD
+

 /*
+ * ScanCKeywordLookup - see if a given word is a keyword
+ *
+ * Returns the token value of the keyword, or -1 if no match.
+ *
 * Do a binary search using plain strcmp() comparison.  This is much like
 * ScanKeywordLookup(), except we want case-sensitive matching.
 */
-const ScanKeyword *
+int
 ScanCKeywordLookup(const char *text)
 {
-	const ScanKeyword *low = &ScanCKeywords[0];
-	const ScanKeyword *high = &ScanCKeywords[lengthof(ScanCKeywords) - 1];
+	const char *kw_string;
+	const uint16 *kw_offsets;
+	const uint16 *low;
+	const uint16 *high;
+
+	if (strlen(text) > ScanCKeywords.max_kw_len)
+		return -1;				/* too long to be any keyword */
+
+	kw_string = ScanCKeywords.kw_string;
+	kw_offsets = ScanCKeywords.kw_offsets;
+	low = kw_offsets;
+	high = kw_offsets + (ScanCKeywords.num_keywords - 1);

 	while (low <= high)
 	{
-		const ScanKeyword *middle;
+		const uint16 *middle;
 		int			difference;

 		middle = low + (high - low) / 2;
-		difference = strcmp(middle->name, text);
+		difference = strcmp(kw_string + *middle, text);
 		if (difference == 0)
-			return middle;
+			return ScanCKeywordTokens[middle - kw_offsets];
 		else if (difference < 0)
 			low = middle + 1;
 		else
 			high = middle - 1;
 	}

-	return NULL;
+	return -1;
 }
--- a/src/interfaces/ecpg/preproc/c_kwlist.h
+++ b/src/interfaces/ecpg/preproc/c_kwlist.h
@@ -0,0 +1,53 @@
+/*-------------------------------------------------------------------------
+ *
+ * c_kwlist.h
+ *
+ * The keyword lists are kept in their own source files for use by
+ * automatic tools.  The exact representation of a keyword is determined
+ * by the PG_KEYWORD macro, which is not defined in this file; it can
+ * be defined by the caller for special purposes.
+ *
+ * Portions Copyright (c) 1996-2019, PostgreSQL Global Development Group
+ * Portions Copyright (c) 1994, Regents of the University of California
+ *
+ * src/interfaces/ecpg/preproc/c_kwlist.h
+ *
+ *-------------------------------------------------------------------------
+ */
+
+/* There is deliberately not an #ifndef C_KWLIST_H here. */
+
+/*
+ * List of (keyword-name, keyword-token-value) pairs.
+ *
+ * !!WARNING!!: This list must be sorted by ASCII name, because binary
+ *		 search is used to locate entries.
+ */
+
+/* name, value */
+PG_KEYWORD("VARCHAR", VARCHAR)
+PG_KEYWORD("auto", S_AUTO)
+PG_KEYWORD("bool", SQL_BOOL)
+PG_KEYWORD("char", CHAR_P)
+PG_KEYWORD("const", S_CONST)
+PG_KEYWORD("enum", ENUM_P)
+PG_KEYWORD("extern", S_EXTERN)
+PG_KEYWORD("float", FLOAT_P)
+PG_KEYWORD("hour", HOUR_P)
+PG_KEYWORD("int", INT_P)
+PG_KEYWORD("long", SQL_LONG)
+PG_KEYWORD("minute", MINUTE_P)
+PG_KEYWORD("month", MONTH_P)
+PG_KEYWORD("register", S_REGISTER)
+PG_KEYWORD("second", SECOND_P)
+PG_KEYWORD("short", SQL_SHORT)
+PG_KEYWORD("signed", SQL_SIGNED)
+PG_KEYWORD("static", S_STATIC)
+PG_KEYWORD("struct", SQL_STRUCT)
+PG_KEYWORD("to", TO)
+PG_KEYWORD("typedef", S_TYPEDEF)
+PG_KEYWORD("union", UNION)
+PG_KEYWORD("unsigned", SQL_UNSIGNED)
+PG_KEYWORD("varchar", VARCHAR)
+PG_KEYWORD("volatile", S_VOLATILE)
+PG_KEYWORD("year", YEAR_P)
--- a/src/interfaces/ecpg/preproc/ecpg_keywords.c
+++ b/src/interfaces/ecpg/preproc/ecpg_keywords.c
@@ -16,82 +16,40 @@
 #include "preproc_extern.h"
 #include "preproc.h"

-/*
- * List of (keyword-name, keyword-token-value) pairs.
- *
- * !!WARNING!!: This list must be sorted, because binary
- *		 search is used to locate entries.
- */
-static const ScanKeyword ECPGScanKeywords[] = {
-	/* name, value, category */
+/* ScanKeywordList lookup data for ECPG keywords */
+#include "ecpg_kwlist_d.h"

-	/*
-	 * category is not needed in ecpg, it is only here so we can share the
-	 * data structure with the backend
-	 */
-	{"allocate", SQL_ALLOCATE, 0},
-	{"autocommit", SQL_AUTOCOMMIT, 0},
-	{"bool", SQL_BOOL, 0},
-	{"break", SQL_BREAK, 0},
-	{"cardinality", SQL_CARDINALITY, 0},
-	{"connect", SQL_CONNECT, 0},
-	{"count", SQL_COUNT, 0},
-	{"datetime_interval_code", SQL_DATETIME_INTERVAL_CODE, 0},
-	{"datetime_interval_precision", SQL_DATETIME_INTERVAL_PRECISION, 0},
-	{"describe", SQL_DESCRIBE, 0},
-	{"descriptor", SQL_DESCRIPTOR, 0},
-	{"disconnect", SQL_DISCONNECT, 0},
-	{"found", SQL_FOUND, 0},
-	{"free", SQL_FREE, 0},
-	{"get", SQL_GET, 0},
-	{"go", SQL_GO, 0},
-	{"goto", SQL_GOTO, 0},
-	{"identified", SQL_IDENTIFIED, 0},
-	{"indicator", SQL_INDICATOR, 0},
-	{"key_member", SQL_KEY_MEMBER, 0},
-	{"length", SQL_LENGTH, 0},
-	{"long", SQL_LONG, 0},
-	{"nullable", SQL_NULLABLE, 0},
-	{"octet_length", SQL_OCTET_LENGTH, 0},
-	{"open", SQL_OPEN, 0},
-	{"output", SQL_OUTPUT, 0},
-	{"reference", SQL_REFERENCE, 0},
-	{"returned_length", SQL_RETURNED_LENGTH, 0},
-	{"returned_octet_length", SQL_RETURNED_OCTET_LENGTH, 0},
-	{"scale", SQL_SCALE, 0},
-	{"section", SQL_SECTION, 0},
-	{"short", SQL_SHORT, 0},
-	{"signed", SQL_SIGNED, 0},
-	{"sqlerror", SQL_SQLERROR, 0},
-	{"sqlprint", SQL_SQLPRINT, 0},
-	{"sqlwarning", SQL_SQLWARNING, 0},
-	{"stop", SQL_STOP, 0},
-	{"struct", SQL_STRUCT, 0},
-	{"unsigned", SQL_UNSIGNED, 0},
-	{"var", SQL_VAR, 0},
-	{"whenever", SQL_WHENEVER, 0},
+/* Token codes for ECPG keywords */
+#define PG_KEYWORD(kwname, value) value,
+
+static const uint16 ECPGScanKeywordTokens[] = {
+#include "ecpg_kwlist.h"
 };

+#undef PG_KEYWORD
+
+
 /*
 * ScanECPGKeywordLookup - see if a given word is a keyword
 *
- * Returns a pointer to the ScanKeyword table entry, or NULL if no match.
+ * Returns the token value of the keyword, or -1 if no match.
+ *
 * Keywords are matched using the same case-folding rules as in the backend.
 */
-const ScanKeyword *
+int
 ScanECPGKeywordLookup(const char *text)
 {
-	const ScanKeyword *res;
+	int			kwnum;

 	/* First check SQL symbols defined by the backend. */
-	res = ScanKeywordLookup(text, SQLScanKeywords, NumSQLScanKeywords);
-	if (res)
-		return res;
+	kwnum = ScanKeywordLookup(text, &ScanKeywords);
+	if (kwnum >= 0)
+		return SQLScanKeywordTokens[kwnum];

 	/* Try ECPG-specific keywords. */
-	res = ScanKeywordLookup(text, ECPGScanKeywords, lengthof(ECPGScanKeywords));
-	if (res)
-		return res;
+	kwnum = ScanKeywordLookup(text, &ScanECPGKeywords);
+	if (kwnum >= 0)
+		return ECPGScanKeywordTokens[kwnum];

-	return NULL;
+	return -1;
 }
--- a/src/interfaces/ecpg/preproc/ecpg_kwlist.h
+++ b/src/interfaces/ecpg/preproc/ecpg_kwlist.h
@@ -0,0 +1,68 @@
+/*-------------------------------------------------------------------------
+ *
+ * ecpg_kwlist.h
+ *
+ * The keyword lists are kept in their own source files for use by
+ * automatic tools.  The exact representation of a keyword is determined
+ * by the PG_KEYWORD macro, which is not defined in this file; it can
+ * be defined by the caller for special purposes.
+ *
+ * Portions Copyright (c) 1996-2019, PostgreSQL Global Development Group
+ * Portions Copyright (c) 1994, Regents of the University of California
+ *
+ * src/interfaces/ecpg/preproc/ecpg_kwlist.h
+ *
+ *-------------------------------------------------------------------------
+ */
+
+/* There is deliberately not an #ifndef ECPG_KWLIST_H here. */
+
+/*
+ * List of (keyword-name, keyword-token-value) pairs.
+ *
+ * !!WARNING!!: This list must be sorted by ASCII name, because binary
+ *		 search is used to locate entries.
+ */
+
+/* name, value */
+PG_KEYWORD("allocate", SQL_ALLOCATE)
+PG_KEYWORD("autocommit", SQL_AUTOCOMMIT)
+PG_KEYWORD("bool", SQL_BOOL)
+PG_KEYWORD("break", SQL_BREAK)
+PG_KEYWORD("cardinality", SQL_CARDINALITY)
+PG_KEYWORD("connect", SQL_CONNECT)
+PG_KEYWORD("count", SQL_COUNT)
+PG_KEYWORD("datetime_interval_code", SQL_DATETIME_INTERVAL_CODE)
+PG_KEYWORD("datetime_interval_precision", SQL_DATETIME_INTERVAL_PRECISION)
+PG_KEYWORD("describe", SQL_DESCRIBE)
+PG_KEYWORD("descriptor", SQL_DESCRIPTOR)
+PG_KEYWORD("disconnect", SQL_DISCONNECT)
+PG_KEYWORD("found", SQL_FOUND)
+PG_KEYWORD("free", SQL_FREE)
+PG_KEYWORD("get", SQL_GET)
+PG_KEYWORD("go", SQL_GO)
+PG_KEYWORD("goto", SQL_GOTO)
+PG_KEYWORD("identified", SQL_IDENTIFIED)
+PG_KEYWORD("indicator", SQL_INDICATOR)
+PG_KEYWORD("key_member", SQL_KEY_MEMBER)
+PG_KEYWORD("length", SQL_LENGTH)
+PG_KEYWORD("long", SQL_LONG)
+PG_KEYWORD("nullable", SQL_NULLABLE)
+PG_KEYWORD("octet_length", SQL_OCTET_LENGTH)
+PG_KEYWORD("open", SQL_OPEN)
+PG_KEYWORD("output", SQL_OUTPUT)
+PG_KEYWORD("reference", SQL_REFERENCE)
+PG_KEYWORD("returned_length", SQL_RETURNED_LENGTH)
+PG_KEYWORD("returned_octet_length", SQL_RETURNED_OCTET_LENGTH)
+PG_KEYWORD("scale", SQL_SCALE)
+PG_KEYWORD("section", SQL_SECTION)
+PG_KEYWORD("short", SQL_SHORT)
+PG_KEYWORD("signed", SQL_SIGNED)
+PG_KEYWORD("sqlerror", SQL_SQLERROR)
+PG_KEYWORD("sqlprint", SQL_SQLPRINT)
+PG_KEYWORD("sqlwarning", SQL_SQLWARNING)
+PG_KEYWORD("stop", SQL_STOP)
+PG_KEYWORD("struct", SQL_STRUCT)
+PG_KEYWORD("unsigned", SQL_UNSIGNED)
+PG_KEYWORD("var", SQL_VAR)
+PG_KEYWORD("whenever", SQL_WHENEVER)
--- a/src/interfaces/ecpg/preproc/keywords.c
+++ b/src/interfaces/ecpg/preproc/keywords.c
@@ -17,24 +17,22 @@

 /*
 * This is much trickier than it looks.  We are #include'ing kwlist.h
- * but the "value" numbers that go into the table are from preproc.h
- * not the backend's gram.h.  Therefore this table will recognize all
- * keywords known to the backend, but will supply the token numbers used
+ * but the token numbers that go into the table are from preproc.h
+ * not the backend's gram.h.  Therefore this token table will match
+ * the ScanKeywords table supplied from common/keywords.c, including all
+ * keywords known to the backend, but it will supply the token numbers used
 * by ecpg's grammar, which is what we need.  The ecpg grammar must
 * define all the same token names the backend does, else we'll get
 * undefined-symbol failures in this compile.
 */

-#include "common/keywords.h"
-
 #include "preproc_extern.h"
 #include "preproc.h"

+#define PG_KEYWORD(kwname, value, category) value,

-#define PG_KEYWORD(a,b,c) {a,b,c},
-
-const ScanKeyword SQLScanKeywords[] = {
+const uint16 SQLScanKeywordTokens[] = {
 #include "parser/kwlist.h"
 };

-const int	NumSQLScanKeywords = lengthof(SQLScanKeywords);
+#undef PG_KEYWORD
--- a/src/interfaces/ecpg/preproc/pgc.l
+++ b/src/interfaces/ecpg/preproc/pgc.l
@@ -920,19 +920,19 @@ cppline			{space}*#([^i][A-Za-z]*|{if}|{ifdef}|{ifndef}|{import})((\/\*[^*/]*\*+
 				}

 {identifier}	{
-					const ScanKeyword  *keyword;
-
 					if (!isdefine())
 					{
+						int		kwvalue;
+
 						/* Is it an SQL/ECPG keyword? */
-						keyword = ScanECPGKeywordLookup(yytext);
-						if (keyword != NULL)
-							return keyword->value;
+						kwvalue = ScanECPGKeywordLookup(yytext);
+						if (kwvalue >= 0)
+							return kwvalue;

 						/* Is it a C keyword? */
-						keyword = ScanCKeywordLookup(yytext);
-						if (keyword != NULL)
-							return keyword->value;
+						kwvalue = ScanCKeywordLookup(yytext);
+						if (kwvalue >= 0)
+							return kwvalue;

 						/*
 						 * None of the above.  Return it as an identifier.
@@ -1010,12 +1010,11 @@ cppline			{space}*#([^i][A-Za-z]*|{if}|{ifdef}|{ifndef}|{import})((\/\*[^*/]*\*+
 						return CPP_LINE;
 					}
 <C>{identifier}		{
-						const ScanKeyword		*keyword;
-
 						/*
 						 * Try to detect a function name:
 						 * look for identifiers at the global scope
-						 * keep the last identifier before the first '(' and '{' */
+						 * keep the last identifier before the first '(' and '{'
+						 */
 						if (braces_open == 0 && parenths_open == 0)
 						{
 							if (current_function)
@@ -1026,9 +1025,11 @@ cppline			{space}*#([^i][A-Za-z]*|{if}|{ifdef}|{ifndef}|{import})((\/\*[^*/]*\*+
 						/* however, some defines have to be taken care of for compatibility */
 						if ((!INFORMIX_MODE || !isinformixdefine()) && !isdefine())
 						{
-							keyword = ScanCKeywordLookup(yytext);
-							if (keyword != NULL)
-								return keyword->value;
+							int		kwvalue;
+
+							kwvalue = ScanCKeywordLookup(yytext);
+							if (kwvalue >= 0)
+								return kwvalue;
 							else
 							{
 								base_yylval.str = mm_strdup(yytext);
--- a/src/interfaces/ecpg/preproc/preproc_extern.h
+++ b/src/interfaces/ecpg/preproc/preproc_extern.h
@@ -59,8 +59,7 @@ extern struct when when_error,
 extern struct ECPGstruct_member *struct_member_list[STRUCT_DEPTH];

 /* Globals from keywords.c */
-extern const ScanKeyword SQLScanKeywords[];
-extern const int NumSQLScanKeywords;
+extern const uint16 SQLScanKeywordTokens[];

 /* functions */

@@ -102,8 +101,8 @@ extern void check_indicator(struct ECPGtype *);
 extern void remove_typedefs(int);
 extern void remove_variables(int);
 extern struct variable *new_variable(const char *, struct ECPGtype *, int);
-extern const ScanKeyword *ScanCKeywordLookup(const char *);
-extern const ScanKeyword *ScanECPGKeywordLookup(const char *text);
+extern int	ScanCKeywordLookup(const char *text);
+extern int	ScanECPGKeywordLookup(const char *text);
 extern void parser_init(void);
 extern int	filtered_base_yylex(void);