Re: reducing the footprint of ScanKeyword (was Re: Large writable variables)
Tom Lane <tgl@sss.pgh.pa.us>
From: Tom Lane <tgl@sss.pgh.pa.us>
To: John Naylor <jcnaylor@gmail.com>
Cc: Andres Freund <andres@anarazel.de>, Robert Haas <robertmhaas@gmail.com>,
Andrew Gierth <andrew@tao11.riddles.org.uk>,
Joerg Sonnenberger <joerg@bec.de>,
"pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Date: 2019-01-06T03:48:51Z
Lists: pgsql-hackers
Attachments
- offset-based-keyword-lookup-7.patch (text/x-diff) patch
John Naylor <jcnaylor@gmail.com> writes: > [ v6-0001-Use-offset-based-keyword-lookup.patch ] I spent some time hacking on this today, and I think it's committable now, but I'm putting it back up in case anyone wants to have another look (and also so the cfbot can check it on Windows). Given the discussion about possibly switching to perfect hashing, I thought it'd be a good idea to try to make the APIs less dependent on the exact table representation. So in the attached, I created a struct ScanKeywordList that holds all the data ScanKeywordLookup needs, and the generated headers declare variables of that type, and we just pass around a pointer to that instead of passing several different things. I also went ahead with the idea of splitting the category and token data into separate arrays. That allows moving the backend token array out of src/common entirely, which I think is a good thing because of the dependency situation: we no longer need to run the bison build before we can compile src/common/keywords_srv.o. There's one remaining refactoring issue that I think we'd want to consider before trying to jack this up and wheel a perfect-hash lookup under it: where to do the downcasing transform. Right now, ecpg's c_keywords.c has its own copy of the binary-search logic because it doesn't want the downcasing transform that ScanKeywordLookup does. So unless we want it to also have a copy of the hash lookup logic, we need to rearrange that somehow. We could give ScanKeywordLookup a "bool downcase" argument, or we could refactor things so that the downcasing is done by callers if they need it (which many don't). I'm not very sure which of those three alternatives is best. My argument upthread that we could always do the downcasing before keyword lookup now feels a bit shaky, because I was reminded while working on this code that we actually have different downcasing rules for keywords and identifiers (yes, really), so that it's not possible for those code paths to share a downcasing transform. So the idea of moving the keyword-downcasing logic to the callers is likely to not work out quite as nicely as I thought. (This might also mean that I was overly hasty to reject Joerg's |0x20 hack. It's still an ugly hack, but it would save doing the keyword downcasing transform if we don't get a hashcode match...) regards, tom lane
Commits
-
Use perfect hashing, instead of binary search, for keyword lookup.
- c64d0cd5ce24 12.0 landed
-
Reduce the size of the fmgr_builtin_oid_index[] array.
- 8ff5f824dca7 12.0 landed
-
Replace the data structure used for keyword lookup.
- afb0d0712f1a 12.0 landed