24 Commits (dd0f37eccecc2db5c0ffafefbf697a2c916e8bc3)

Author	SHA1	Message	Date
Michael Paquier	dd0f37ecce	Fix collection of typos and grammar mistakes in the tree ... This fixes some comments and documentation new as of Postgres 13. Author: Justin Pryzby Discussion: https://postgr.es/m/20200408165653.GF2228@telsasoft.com	6 years ago
David Rowley	d025cf88ba	Modify various power 2 calculations to use new helper functions ... First pass of modifying various places that obtain the next power of 2 of a number and make them use the new functions added in pg_bitutils.h instead. This also removes the _hash_log2() function. There are no longer any callers in core. Other users can swap their _hash_log2(n) call to make use of pg_ceil_log2_32(n). Author: David Fetter, with some minor adjustments by me Reviewed-by: John Naylor, Jesse Zhang Discussion: https://postgr.es/m/20200114173553.GE32763%40fetter.org	6 years ago
Robert Haas	da41d71070	simplehash: Allow for use in frontend code. ... Commit 48995040d5 removed the largest barrier to use of simplehash in frontend code, but there's one more problem: it uses elog(ERROR, ...) or elog(LOG, ...) in a couple of places. Work around that by changing those to pg_log_error() and pg_log_info() when FRONTEND is defined. Patch by me, reviewed by Andres Freund. Discussion: http://postgr.es/m/CA+Tgmob8oyh02NrZW=xCScB+5GyJ-jVowE3+TWTUmPF=FsGWTA@mail.gmail.com	6 years ago
Robert Haas	48995040d5	simplehash: Allow use of simplehash without MemoryContext. ... If the SH_RAW_ALLOCATOR is defined, it will be used to allocate bytes for the hash table, and no dependencies on MemoryContext will exist. This means, in particular, that the SH_CREATE function will not take a MemoryContext argument. Patch by me, reviewed by Andres Freund. Discussion: http://postgr.es/m/CA+Tgmob8oyh02NrZW=xCScB+5GyJ-jVowE3+TWTUmPF=FsGWTA@mail.gmail.com	6 years ago
Jeff Davis	6ae4e8eae7	Allow simplehash to use already-calculated hash values. ... Add _lookup_hash and _insert_hash functions for callers that have already calculated the hash value of the key. The immediate use case is for hash algorithms that write to disk in partitions. The hash value can be calculated once, used to perform a lookup, used to select the partition, then written to the partition along with the tuple. When the tuple is read back, the hash value does not need to be recalculated. Author: Jeff Davis Reviewed-by: Andres Freund Discussion: https://postgr.es/m/48abe675e1330f0c264ab2fe0d4ff23eb244f9ef.camel%40j-davis.com	7 years ago
Tom Lane	8255c7a5ee	Phase 2 pgindent run for v12. ... Switch to 2.1 version of pg_bsd_indent. This formats multiline function declarations "correctly", that is with additional lines of parameter declarations indented to match where the first line's left parenthesis is. Discussion: https://postgr.es/m/CAEepm=0P3FeTXRcU5B2W3jv3PgRVZ-kGUXLGfd42FFhUROO3ug@mail.gmail.com	7 years ago
Andres Freund	3b632a58e7	simplehash: Add support for resetting a hashtable's contents. ... A hashtable reset just reset the hashtable entries, but does not free memory. Author: Andres Freund Discussion: https://postgr.es/m/20190114180423.ywhdg2iagzvh43we@alap3.anarazel.de Bug: #15592 #15486 Backpatch: 11, this is a prerequisite for other fixes	7 years ago
Thomas Munro	ee0e2745c2	Code review for simplehash.h. ... Fix reference to non-existent file in comment. Add SH_ prefix to the EMPTY and IN_USE tokens, to reduce likelihood of collisions with unrelated macros. Add include guards around the function definitions that are not "parameterized", so the header can be used again in the same translation unit. Undefine SH_EQUAL macro where other "parameter" macros are undefined, for the same reason. Author: Thomas Munro Reviewed-by: Tom Lane Discussion: https://postgr.es/m/CAEepm%3D1LdXZ3mMTM8tHt_b%3DK1kREit%3Dp8sikesak%3DkzHHM07Nw%40mail.gmail.com	7 years ago
Andres Freund	ab9f2c429d	Prevent growth of simplehash tables when they're "too empty". ... In cases where simplehash tables where filled with either a lot of conflicting hash-values, or values that hash to consecutive values (i.e. build "chains") the growth heuristics in d4c62a6b62 could trigger rather explosively. To fix that, address some of the reasons (see previous commit) of why the growth heuristics where needed, and only allow growth when the table isn't too empty. While that means there's a few cases of bad input that can be slower, that seems a lot better than running very quickly out of memory. Author: Tomas Vondra and Andres Freund, with additional input by Thomas Munro, Tom Lane Todd A. Cook Reported-By: Todd A. Cook, Tomas Vondra, Thomas Munro Discussion: https://postgr.es/m/20171127185700.1470.20362@wrigleys.postgresql.org Backpatch: 10, where simplehash was introduced	8 years ago
Tom Lane	c7b8998ebb	Phase 2 of pgindent updates. ... Change pg_bsd_indent to follow upstream rules for placement of comments to the right of code, and remove pgindent hack that caused comments following #endif to not obey the general rule. Commit e3860ffa4d wasn't actually using the published version of pg_bsd_indent, but a hacked-up version that tried to minimize the amount of movement of comments to the right of code. The situation of interest is where such a comment has to be moved to the right of its default placement at column 33 because there's code there. BSD indent has always moved right in units of tab stops in such cases --- but in the previous incarnation, indent was working in 8-space tab stops, while now it knows we use 4-space tabs. So the net result is that in about half the cases, such comments are placed one tab stop left of before. This is better all around: it leaves more room on the line for comment text, and it means that in such cases the comment uniformly starts at the next 4-space tab stop after the code, rather than sometimes one and sometimes two tabs after. Also, ensure that comments following #endif are indented the same as comments following other preprocessor commands such as #else. That inconsistency turns out to have been self-inflicted damage from a poorly-thought-through post-indent "fixup" in pgindent. This patch is much less interesting than the first round of indent changes, but also bulkier, so I thought it best to separate the effects. Discussion: https://postgr.es/m/E1dAmxK-0006EE-1r@gemulon.postgresql.org Discussion: https://postgr.es/m/30527.1495162840@sss.pgh.pa.us	9 years ago
Tom Lane	e3860ffa4d	Initial pgindent run with pg_bsd_indent version 2.0. ... The new indent version includes numerous fixes thanks to Piotr Stefaniak. The main changes visible in this commit are: * Nicer formatting of function-pointer declarations. * No longer unexpectedly removes spaces in expressions using casts, sizeof, or offsetof. * No longer wants to add a space in "struct structname *varname", as well as some similar cases for const- or volatile-qualified pointers. * Declarations using PG_USED_FOR_ASSERTS_ONLY are formatted more nicely. * Fixes bug where comments following declarations were sometimes placed with no space separating them from the code. * Fixes some odd decisions for comments following case labels. * Fixes some cases where comments following code were indented to less than the expected column 33. On the less good side, it now tends to put more whitespace around typedef names that are not listed in typedefs.list. This might encourage us to put more effort into typedef name collection; it's not really a bug in indent itself. There are more changes coming after this round, having to do with comment indentation and alignment of lines appearing within parentheses. I wanted to limit the size of the diffs to something that could be reviewed without one's eyes completely glazing over, so it seemed better to split up the changes as much as practical. Discussion: https://postgr.es/m/E1dAmxK-0006EE-1r@gemulon.postgresql.org Discussion: https://postgr.es/m/30527.1495162840@sss.pgh.pa.us	9 years ago
Magnus Hagander	bb1f8f9e5b	Fix typos in comments ... Author: Daniel Gustafsson <daniel@yesql.se>	9 years ago
Heikki Linnakangas	553e16951c	Fix comments in simplehash.h. ... Jeff Janes and me. Discussion: https://www.postgresql.org/message-id/CAMkU=1zYnniLYg+W9itL93DXebCjx6Uk6m_=Xa8p_zM65X3S0Q@mail.gmail.com	9 years ago
Bruce Momjian	a6fd7b7a5f	Post-PG 10 beta1 pgindent run ... perltidy run not included.	9 years ago
Andres Freund	d4c62a6b62	Make simplehash.h grow hashtable in additional cases. ... Increase the size when either the distance between actual and optimal slot grows too large, or when too many subsequent entries would have to be moved. This addresses reports that the simplehash performed, sometimes considerably, worse than dynahash. The reason turned out to be that insertions into the hashtable where, due to the use of parallel query, in effect done from another hashtable, in hash-value order. If the target hashtable, due to mis-estimation, was sized a lot smaller than the source table(s) that lead to very imbalanced tables; a lot of entries in many close-by buckets from the source tables were inserted into a single, wider, bucket on the target table. As the growth factor was solely computed based on the fillfactor, the performance of the table decreased further and further. b81b5a96f4 was an attempt to address this problem for hash aggregates (but not for bitmap scans), but it turns out that the current method of mixing hash values often actually leaves neighboring hash-values close to each other, just in different value range. It might be worth revisiting that independently of the performance issues addressed in this patch.. To address that problem resize tables in two additional cases: Firstly when the optimal position for an entry would be far from the actual position, secondly when many entries would have to be moved to make space for the new entry (while satisfying the robin hood property). Due to the additional resizing threshold it seems possible, and testing confirms that so far, that a higher fillfactor doesn't hurt performance and saves a bit of memory. It seems better to increase it now, before a release containing any of this code, rather than wonder in some later release. The various boundaries aren't determined in a particularly scientific manner, they might need some fine-tuning. In all my tests the new code now, even with parallelism, performs at least as good as the old code, in several scenarios significantly better. Reported-By: Dilip Kumar, Robert Haas, Kuntal Ghosh Discussion: https://postgr.es/m/CAFiTN-vagvuAydKG9VnWcoK=ADAhxmOa4ZTrmNsViBBooTnriQ@mail.gmail.com https://postgr.es/m/CAGz5QC+=fNTYgzMLTBUNeKt6uaWZFXJbkB5+7oWm-n9DwVxcLA@mail.gmail.com	9 years ago
Andres Freund	8f7277dfb5	Fix s/ITERTOR/ITERATOR/ typo in simplehash.h. ... This could lead to problem when simplehash.h is used to define two different types of hashtable visible in the same translation unit. Reported-By: Josh Soref Discussion: https://postgr.es/m/CACZqfqCC7WdBAY=rQePb9-qW1rjdaTdHsV5KoVejHkDb6qrtOg@mail.gmail.com	9 years ago
Robert Haas	72257f9578	simplehash: Additional tweaks to make specifying an allocator work. ... Even if we don't emit definitions for SH_ALLOCATE and SH_FREE, we still need prototypes. The user can't define them before including simplehash.h because SH_TYPE isn't available yet. For the allocator to be able to access private_data, it needs to become an argument to SH_CREATE. Previously we relied on callers to set that after returning from SH_CREATE, but SH_CREATE calls SH_ALLOCATE before returning. Dilip Kumar, reviewed by me.	9 years ago
Robert Haas	c3c4f6e174	Revise the way the element allocator for a simplehash is specified. ... This method is more elegant and more efficient. Per a suggestion from Andres Freund, who also briefly reviewed the patch.	9 years ago
Robert Haas	ac8eb972f2	Avoid redefining simplehash_allocate/simplehash_free. ... There's no generic guard against multiple inclusion in this file, for good reason. But these typedefs need one, as per a report from Jeff Janes.	9 years ago
Robert Haas	565903af47	Allow the element allocator for a simplehash to be specified. ... This is infrastructure for a pending patch to allow parallel bitmap heap scans. Dilip Kumar, reviewed (in earlier versions) by Andres Freund and (more recently) by me. Some further renaming by me, also.	9 years ago
Heikki Linnakangas	181bdb90ba	Fix typos in comments. ... Backpatch to all supported versions, where applicable, to make backpatching of future fixes go more smoothly. Josh Soref Discussion: https://www.postgresql.org/message-id/CACZqfqCf+5qRztLPgmmosr-B0Ye4srWzzw_mo4c_8_B_mtjmJQ@mail.gmail.com	9 years ago
Peter Eisentraut	c32fe432af	Avoid using a C++ keyword in header file ... per cpluspluscheck	9 years ago
Andres Freund	90d3da11c9	Fix a few typos in simplehash.h. ... Author: Erik Rijkers Discussion: <274e4c8ac545d6622735f97c1f6c354b@xs4all.nl>	9 years ago
Andres Freund	b30d3ea824	Add a macro templatized hashtable. ... dynahash.c hash tables aren't quite fast enough for some use-cases. There are several reasons for lacking performance: - the use of chaining for collision handling makes them cache inefficient, that's especially an issue when the tables get bigger. - as the element sizes for dynahash are only determined at runtime, offset computations are somewhat expensive - hash and element comparisons are indirect function calls, causing unnecessary pipeline stalls - it's two level structure has some benefits (somewhat natural partitioning), but increases the number of indirections to fix several of these the hash tables have to be adjusted to the individual use-case at compile-time. C unfortunately doesn't provide a good way to do compile code generation (like e.g. c++'s templates for all their weaknesses do). Thus the somewhat ugly approach taken here is to allow for code generation using a macro-templatized header file, which generates functions and types based on a prefix and other parameters. Later patches use this infrastructure to use such hash tables for tidbitmap.c (bitmap scans) and execGrouping.c (hash aggregation, ...). In queries where these use up a large fraction of the time, this has been measured to lead to performance improvements of over 100%. There are other cases where this could be useful (e.g. catcache.c). The hash table design chosen is a variant of linear open-addressing. The biggest disadvantage of simple linear addressing schemes are highly variable lookup times due to clustering, and deletions leaving a lot of tombstones around. To address these issues a variant of "robin hood" hashing is employed. Robin hood hashing optimizes chaining lengths by moving elements close to their optimal bucket ("rich" elements), out of the way if a to-be-inserted element is further away from its optimal position (i.e. it's "poor"). While that can make insertions slower, the average lookup performance is a lot better, and higher fill factors can be used in a still performant manner. To avoid tombstones - which normally solve the issue that a deleted node's presence is relevant to determine whether a lookup needs to continue looking or is done - buckets following a deleted element are shifted backwards, unless they're empty or already at their optimal position. There's further possible improvements that can be made to this implementation. Amongst others: - Use distance as a termination criteria during searches. This is generally a good idea, but I've been able to see the overhead of distance calculations in some cases. - Consider combining the 'empty' status into the hashvalue, and enforce storing the hashvalue. That could, in some cases, increase memory density and remove a few instructions. - Experiment further with the, very conservatively choosen, fillfactor. - Make maximum size of hashtable configurable, to allow storing very very large tables. That'd require 64bit hash values to be more common than now, though. - some smaller memcpy calls could be optimized to copy larger chunks But since the new implementation is already considerably faster than dynahash it seem sensible to start using it. Reviewed-By: Tomas Vondra Discussion: <20160727004333.r3e2k2y6fvk2ntup@alap3.anarazel.de>	9 years ago