Re: Comment simplehash/dynahash trade-offs

Thomas Munro <thomas.munro@gmail.com>

From: Thomas Munro <thomas.munro@gmail.com>
To: James Coleman <jtc331@gmail.com>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>, Tomas Vondra <tomas.vondra@2ndquadrant.com>, Andres Freund <andres@anarazel.de>
Date: 2020-07-20T05:28:53Z
Lists: pgsql-hackers
On Fri, May 1, 2020 at 1:53 PM James Coleman <jtc331@gmail.com> wrote:
> In another thread [1] I'd mused that "there might be some value in a
> README or comments
> addition that would be a guide to what the various hash
> implementations are useful for...so that we have something to make the
> code base a bit more discoverable."

+1

> I'd solicited feedback from Andres (as the author of the simplehash
> implementation) and gotten further explanation from Tomas (both cc'd
> here) and have tried to condense that into the comment changes in this
> patch series.
>
> v1-0001-Summarize-trade-offs-between-simplehash-and-dynah.patch
> Contains the summaries mentioned above.

+ * - It supports partitioning, which is useful for shared memory access using

I wonder if we should say a bit more about the shared memory mode.
Shared memory dynahash tables are allocated in a fixed size area at
startup, and are discoverable by name in other other processes that
need to get access to them, while simplehash assumes that it can get
memory from a MemoryContext or an allocator with a malloc/free-style
interface, which isn't very well suited for use in shared memory.
(I'm sure you can convince it to work in shared memory with some
work.)

> v1-0002-Improve-simplehash-usage-notes.patch

+ *    For convenience the hash table create functions accept a void pointer
+ *    will be stored in the hash table type's member private_data.

*that* will be stored?

> v1-0003-Show-sample-simplehash-method-signatures.patch
> I find it hard to read the macro code "templating" particularly for
> seeing what the available API is and so added sample method signatures
> in comments to the macro generated method signature defines.

I didn't double-check all the expansions of the macros but +1 for this
idea, it's very useful.



Commits

  1. Correct comment in simplehash.h.

  2. Improve programmer docs for simplehash and dynahash.