Re: Change GUC hashtable to use simplehash?

John Naylor <johncnaylorls@gmail.com>

From: John Naylor <johncnaylorls@gmail.com>
To: Jeff Davis <pgsql@j-davis.com>
Cc: Andres Freund <andres@anarazel.de>, Tom Lane <tgl@sss.pgh.pa.us>, Gurjeet Singh <gurjeet@singh.im>, pgsql-hackers@postgresql.org
Date: 2023-12-19T09:23:29Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →
  1. Silence warning in older versions of Valgrind

  2. Revert "Speed up tail processing when hashing aligned C strings, take two"

  3. Speed up tail processing when hashing aligned C strings, take two

  4. Teach fasthash_accum to use platform endianness for bytewise loads

  5. Add macro to disable address safety instrumentation

  6. Convert uses of hash_string_pointer to fasthash equivalent

  7. Speed up tail processing when hashing aligned C strings

  8. Add helper functions for dshash tables with string keys.

  9. Fix warnings in cpluspluscheck

  10. Further cosmetic review of hashfn_unstable.h

  11. Simplify initialization of incremental hash state

  12. Add optimized C string hashing

  13. Add inline incremental hash functions for in-memory use

  14. Make all Perl warnings fatal

On Tue, Dec 19, 2023 at 2:32 PM Jeff Davis <pgsql@j-davis.com> wrote:
>
> On Mon, 2023-12-18 at 13:39 +0700, John Naylor wrote:
> > For now just two:
> > v10-0002 is Jeff's change to the search path cache, but with the
> > chunked interface that I found to be faster.
>
> Did you consider specializing for the case of an aligned pointer? If
> it's a string (c string or byte string) it's almost always going to be
> aligned, right?

That wasn't the next place I thought to look (that would be the strcmp
call), but something like this could be worthwhile.

If we went this far, I'd like to get more use out of it than one call
site. I think a few other places have as their hash key a string along
with other values, so maybe we can pass an initialized hash state for
strings separately from combining in the other values. Dynahash will
still need to deal with truncation, so would need duplicate coding,
but I'm guessing with that truncation check it's makes an optimization
like you propose even more worthwhile.

> I hacked up a patch (attached). I lost track of which benchmark we're
> using to test the performance, but when I test in a loop it seems
> substantially faster.

That's interesting. Note that there is no need for a new
fasthash_accum64(), since we can do

fasthash_accum(&hs, buf, FH_SIZEOF_ACCUM);

...and the compiler should elide the switch statement.

> It reads past the NUL byte, but only to the next alignment boundary,
> which I think is OK (though I think I'd need to fix the patch for when
> maxalign < 8).

Seems like it, on both accounts.