pgsql: Replace the data structure used for keyword lookup.

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: pgsql-committers(at)lists(dot)postgresql(dot)org
Subject: pgsql: Replace the data structure used for keyword lookup.
Date: 2019-01-06 22:03:34
Message-ID: E1ggGW6-00038A-Ni@gemulon.postgresql.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers

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

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/afb0d0712f1a62efe2addd95262cf38e8481e84a

Modified Files
--------------
contrib/pg_stat_statements/pg_stat_statements.c | 4 +-
src/backend/parser/parser.c | 2 +-
src/backend/parser/scan.l | 51 +++++---
src/backend/utils/adt/misc.c | 8 +-
src/backend/utils/adt/ruleutils.c | 6 +-
src/common/.gitignore | 1 +
src/common/Makefile | 26 ++--
src/common/keywords.c | 99 ++------------
src/common/kwlookup.c | 94 ++++++++++++++
src/fe_utils/string_utils.c | 6 +-
src/include/common/keywords.h | 25 +---
src/include/common/kwlookup.h | 40 ++++++
src/include/parser/kwlist.h | 2 +-
src/include/parser/scanner.h | 13 +-
src/interfaces/ecpg/preproc/.gitignore | 2 +
src/interfaces/ecpg/preproc/Makefile | 22 +++-
src/interfaces/ecpg/preproc/c_keywords.c | 75 +++++------
src/interfaces/ecpg/preproc/c_kwlist.h | 53 ++++++++
src/interfaces/ecpg/preproc/ecpg_keywords.c | 84 +++---------
src/interfaces/ecpg/preproc/ecpg_kwlist.h | 68 ++++++++++
src/interfaces/ecpg/preproc/keywords.c | 16 +--
src/interfaces/ecpg/preproc/pgc.l | 29 +++--
src/interfaces/ecpg/preproc/preproc_extern.h | 7 +-
src/pl/plpgsql/src/.gitignore | 2 +
src/pl/plpgsql/src/Makefile | 19 ++-
src/pl/plpgsql/src/pl_reserved_kwlist.h | 53 ++++++++
src/pl/plpgsql/src/pl_scanner.c | 165 ++++--------------------
src/pl/plpgsql/src/pl_unreserved_kwlist.h | 111 ++++++++++++++++
src/tools/gen_keywordlist.pl | 156 ++++++++++++++++++++++
src/tools/msvc/Mkvcbuild.pm | 2 +-
src/tools/msvc/Solution.pm | 36 ++++++
src/tools/msvc/clean.bat | 5 +
32 files changed, 843 insertions(+), 439 deletions(-)

Browse pgsql-committers by date

  From Date Subject
Next Message Amit Khandekar 2019-01-07 11:33:20 Re: pgsql: Remove WITH OIDS support, change oid catalog column visibility.
Previous Message Michael Paquier 2019-01-05 03:50:24 pgsql: doc: Update RFC URLs