ResourceOwner refactoring
Heikki Linnakangas <hlinnaka@iki.fi>
From: Heikki Linnakangas <hlinnaka@iki.fi>
To: pgsql-hackers <pgsql-hackers@postgresql.org>
Cc: Michael Paquier <michael@paquier.xyz>,
Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Date: 2020-11-17T14:21:29Z
Lists: pgsql-hackers
Attachments
- v1-resowner-refactor.patch (text/x-patch) patch v1
Two recent patches that I have reviewed have reminded me that the ResourceOwner interface is a bit clunky. There are two issues: 1. Performance. It's fast enough in most use, but when I was testing Kyotaro's catcache patches [1], the Resowner functions to track catcache references nevertheless showed up, accounting for about 20% of the CPU time used by catcache lookups. 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? Attached patch refactors the ResourceOwner internals to do that. The current code in HEAD has a separate array for each "kind" of object that can be tracked. The number of different kinds of objects has grown over the years, currently there is support for tracking buffers, files, catcache, relcache and plancache references, tupledescs, snapshots, DSM segments and LLVM JIT contexts. And locks, which are a bit special. In the patch, I'm using the same array to hold all kinds of objects, and carry another field with each tracked object, to tell what kind of an object it is. An extension can define a new object kind, by defining Release and PrintLeakWarning callback functions for it. The code in resowner.c is now much more generic, as it doesn't need to know about all the different object kinds anymore (with a couple of exceptions). In the new scheme, there is one small fixed-size array to hold a few most recently-added references, that is linear-searched, and older entries are moved to a hash table. 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? [1] https://www.postgresql.org/message-id/20201106.172958.1103727352904717607.horikyota.ntt%40gmail.com [2] https://www.postgresql.org/message-id/20201113031429.GB1631@paquier.xyz - Heikki
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