Re: ResourceOwner refactoring - Mailing list pgsql-hackers

From Michael Paquier
Subject Re: ResourceOwner refactoring
Date
Msg-id 20201118080648.GL19692@paquier.xyz
Whole thread Raw
In response to ResourceOwner refactoring  (Heikki Linnakangas <hlinnaka@iki.fi>)
Responses Re: ResourceOwner refactoring  (Heikki Linnakangas <hlinnaka@iki.fi>)
List pgsql-hackers
On Tue, Nov 17, 2020 at 04:21:29PM +0200, Heikki Linnakangas wrote:
> 2. It's difficult for extensions to use. There is a callback mechanism for
> extensions, but it's much less convenient to use than the built-in
> functions. The pgcrypto code uses the callbacks currently, and Michael's
> patch [2] would move that support for tracking OpenSSL contexts to the core,
> which makes it a lot more convenient for pgcrypto. Wouldn't it be nice if
> extensions could have the same ergonomics as built-in code, if they need to
> track external resources?

+1.  True that the current interface is a bit hard to grasp, one can
easily miss that showing reference leaks is very important if an
allocation happens out of the in-core palloc machinery at commit time.
(The patch you are referring to is not really popular, but that does
not prevent this thread to move on on its own.)

> Attached patch refactors the ResourceOwner internals to do that.

+ * Size of the small fixed-size array to hold most-recently remembered resources.
  */
-#define RESARRAY_INIT_SIZE 16
+#define RESOWNER_ARRAY_SIZE 8
Why did you choose this size for the initial array?

+extern const char *ResourceOwnerGetName(ResourceOwner owner);
This is only used in resowner.c, at one place.

@@ -140,7 +139,6 @@ jit_release_context(JitContext *context)
    if (provider_successfully_loaded)
            provider.release_context(context);

-   ResourceOwnerForgetJIT(context->resowner, PointerGetDatum(context));
[...]
+   ResourceOwnerForget(context->resowner, PointerGetDatum(context), &jit_funcs);
This moves the JIT context release from jit.c to llvm.c.  I think
that's indeed more consistent, and correct.  It would be better to
check this one with Andres, though.

> I haven't done thorough performance testing of this, but with some quick
> testing with Kyotaro's "catcachebench" to stress-test syscache lookups, this
> performs a little better. In that test, all the activity happens in the
> small array portion, but I believe that's true for most use cases.



> Thoughts? Can anyone suggest test scenarios to verify the performance of
> this?

Playing with catcache.c is the first thing that came to my mind.
After that some micro-benchmarking with DSM or snapshots?  I am not
sure if we would notice a difference in a real-life scenario, say even
a pgbench -S with prepared queries.
--
Michael

Attachment

pgsql-hackers by date:

Previous
From: Peter Smith
Date:
Subject: Re: [HACKERS] logical decoding of two-phase transactions
Next
From: Daniel Gustafsson
Date:
Subject: Re: Move OpenSSL random under USE_OPENSSL_RANDOM