On Sat, Oct 1, 2016 at 2:44 AM, Andres Freund <and...@anarazel.de> wrote:
> Hi, > > On 2016-07-26 17:43:33 -0700, Andres Freund wrote: > > In the attached patch I've attached simplehash.h, which can be > > customized by a bunch of macros, before being inlined. There's also a > > patch using this for tidbitmap.c and nodeAgg/nodeSubplan/... via > > execGrouping.c. > > Attached is a significantly improved version of this series. The main > changes are: > > - The hash table now uses robin-hood style hash collision handling. See the > commit message of 0002 (or simplehash.h) for an introduction. That > significantly reduces the impact of "clustering" due to linear > addressing. > - Significant comment and correctness fixes, both in simplehash.h > - itself, and 0003/0004. > - a lot of other random performance improvements for the hashing code. > > > Unfortunately I'm running out battery right now, so I don't want to > re-run the benchmarks posted upthread (loading takes a while). But the > last time I ran them all the results after the patches were better than > before. > > > This patch series currently consists out of four patches: > - 0001 - add unlikely/likely() macros. The use here is not entirely > mandatory, but they do provide a quite consistent improvement. There > are other threads where they proved to be beneficial, so I see little > reason not to introduce them. > - 0002 - the customizable hashtable itself. It now contains documentation. > - 0003 - convert tidbitmap.c to use the new hashmap. This includes a fix > for the issue mentioned in [1], to improve peformance in heavily lossy > scenarios (otherwise we could regress in some extreme cases) > - 0004 - convert execGrouping.c, e.g. used by hash aggregates > > > While not quite perfect yet, I do think this is at a state where input > is needed. I don't want to go a lot deeper into this rabbit hole, > before we have some agreement that this is a sensible course of action. > > > I think there's several possible additional users of simplehash.h, > e.g. catcache.c - which has an open coded, not particularly fast hash > table and frequently shows up in profiles - but I think the above two > conversions are plenty to start with. > > > Comments? > > > Greetings, > > Andres Freund > > [1] http://archives.postgresql.org/message-id/20160923205843.zcs > 533sqdzlh4cpo%40alap3.anarazel.de > > > -- > Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgsql-hackers > > This is a great addition. A couple of comments. * 80% occupancy is a bit conservative for RH hashing, it works well up to 90% if you use the early stops due to distance. So that TODO is worth pursuing. * An optimized memory layout for RH hashmap is along the lines of HHHKVKVKV, using one bit of the hash as the present/absent flag. That plays specially well with large occupancies (~90%). Due to RH the probings on the H array are very short and within a single cache line. Even with a 31bit hash it's reallyyy rare to have to probe more than 1 entry in the KV array. Essentially guaranteeing that the 99% percentile of lookups will only hit a couple of cache lines (if you ignore crossing cache lines and other details).
