Horton

Re: ResourceOwner refactoring

Julien Rouhaud <rjuju123@gmail.com>

View thread View on postgr.es
From: Julien Rouhaud <rjuju123@gmail.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: Heikki Linnakangas <hlinnaka@iki.fi>, pgsql-hackers <pgsql-hackers@postgresql.org>, Kyotaro Horiguchi <horikyota.ntt@gmail.com>, Andres Freund <andres@anarazel.de>
Date: 2020-11-19T09:27:18Z
Lists: pgsql-hackers 
On Thu, Nov 19, 2020 at 10:16 AM Michael Paquier <michael@paquier.xyz> wrote:
>
> On Wed, Nov 18, 2020 at 10:50:08AM +0200, Heikki Linnakangas wrote:
> > If RESOWNER_ARRAY_STATS is increased to 16, all the lookups fit in the
> > array. But I haven't done any benchmarking to see which is faster.
>
> My gut tells me that your guess is right, but it would be better to be
> sure.
>
> > BTW, I think there would be an easy win in the hashing codepath, by changing
> > to a cheaper hash function. Currently, with or without this patch, we use
> > hash_any(). Changing that to murmurhash32() or something similar would be a
> > drop-in replacement.
>
> Good idea.

+1, and +1 for this refactoring.

I just saw a minor issue in a comment while reviewing the patch:

[...]
+ /* Is there space in the hash? If not, enlarge it. */

  /* Double the capacity of the array (capacity must stay a power of 2!) */
- newcap = (oldcap > 0) ? oldcap * 2 : RESARRAY_INIT_SIZE;
[...]

The existing comment is kept as-is, but it should mention that it's
now the hash capacity that is increased.

Commits

  1. Make RelationFlushRelation() work without ResourceOwner during abort

  2. Fix bug in bulk extending temp relation after failure

  3. Add missing PGDLLIMPORT markings

  4. Add test_dsa module.

  5. Clear CurrentResourceOwner earlier in CommitTransaction.

  6. Fix dsa.c with different resource owners.

  7. Fix bug in the new ResourceOwner implementation.

  8. Change pgcrypto to use the new ResourceOwner mechanism.

  9. Use a faster hash function in resource owners.

  10. Make ResourceOwners more easily extensible.

  11. Move a few ResourceOwnerEnlarge() calls for safety and clarity.