Re: ResourceOwner refactoring
Michael Paquier <michael@paquier.xyz>
From: Michael Paquier <michael@paquier.xyz>
To: Heikki Linnakangas <hlinnaka@iki.fi>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>, Kyotaro Horiguchi <horikyota.ntt@gmail.com>, Andres Freund <andres@anarazel.de>
Date: 2020-11-18T08:06:48Z
Lists: 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
Commits
-
Make RelationFlushRelation() work without ResourceOwner during abort
- e6cd85772647 17.0 landed
-
Fix bug in bulk extending temp relation after failure
- d212957254de 17.0 landed
-
Add missing PGDLLIMPORT markings
- c6b86eaa55ff 17.0 landed
-
Add test_dsa module.
- 325f54033e59 17.0 landed
-
Clear CurrentResourceOwner earlier in CommitTransaction.
- c21e6e2fd48c 17.0 landed
-
Fix dsa.c with different resource owners.
- a8b330ffb6f7 17.0 landed
-
Fix bug in the new ResourceOwner implementation.
- 8f4a1ab471e6 17.0 landed
-
Change pgcrypto to use the new ResourceOwner mechanism.
- cd694f60dc97 17.0 landed
-
Use a faster hash function in resource owners.
- 954e43564d99 17.0 landed
-
Make ResourceOwners more easily extensible.
- b8bff07daa85 17.0 landed
-
Move a few ResourceOwnerEnlarge() calls for safety and clarity.
- b70c2143bbbe 17.0 landed