Re: ResourceOwner refactoring

2024-02-02 Thread Heikki Linnakangas
On 02/02/2024 11:00, Alexander Lakhin wrote: Please try the following script: mkdir /tmp/50m sudo mount -t tmpfs -o size=50M tmpfs /tmp/50m export PGDATA=/tmp/50m/tmpdb initdb pg_ctl -l server.log start cat << 'EOF' | psql CREATE TEMP TABLE t (a name, b name, c name, d name); INSERT INTO t

Re: ResourceOwner refactoring

2024-02-02 Thread Alexander Lakhin
Hello Heikki, 08.11.2023 14:37, Heikki Linnakangas wrote: Fixed these, and pushed. Thanks everyone for reviewing! Please try the following script: mkdir /tmp/50m sudo mount -t tmpfs -o size=50M tmpfs /tmp/50m export PGDATA=/tmp/50m/tmpdb initdb pg_ctl -l server.log start cat << 'EOF' |

Re: ResourceOwner refactoring

2024-01-16 Thread Anton A. Melnikov
On 16.01.2024 14:54, Heikki Linnakangas wrote: Fixed, thanks. mark_pgdllimport.pl also highlighted two new variables in walsummarizer.h, fixed those too. Thanks! Have a nice day! -- Anton A. Melnikov Postgres Professional: http://www.postgrespro.com The Russian Postgres Company

Re: ResourceOwner refactoring

2024-01-16 Thread Heikki Linnakangas
On 16/01/2024 13:23, Anton A. Melnikov wrote: Hello! Found possibly missing PGDLLIMPORT in the definition of the extern const ResourceOwnerDesc buffer_io_resowner_desc; and extern const ResourceOwnerDesc buffer_pin_resowner_desc; callbacks in the src/include/storage/buf_internals.h Please

Re: ResourceOwner refactoring

2024-01-16 Thread Anton A. Melnikov
Hello! Found possibly missing PGDLLIMPORT in the definition of the extern const ResourceOwnerDesc buffer_io_resowner_desc; and extern const ResourceOwnerDesc buffer_pin_resowner_desc; callbacks in the src/include/storage/buf_internals.h Please take a look on it. With the best regards, --

Re: ResourceOwner refactoring

2023-11-17 Thread Andres Freund
Hi, On 2023-11-17 12:44:41 -0800, Andres Freund wrote: > On 2023-11-07 13:28:28 +0200, Heikki Linnakangas wrote: > > I feel pretty good about this overall. Barring objections or new cfbot > > failures, I will commit this in the next few days. > > I am working on rebasing the AIO patch over this.

Re: ResourceOwner refactoring

2023-11-17 Thread Andres Freund
Hi, On 2023-11-07 13:28:28 +0200, Heikki Linnakangas wrote: > I feel pretty good about this overall. Barring objections or new cfbot > failures, I will commit this in the next few days. I am working on rebasing the AIO patch over this. I think I found a crash that's unrelated to AIO. #4

Re: ResourceOwner refactoring

2023-11-15 Thread Heikki Linnakangas
On 13/11/2023 01:08, Thomas Munro wrote: On Mon, Nov 13, 2023 at 11:16 AM Heikki Linnakangas wrote: On 11/11/2023 14:00, Alexander Lakhin wrote: 10.11.2023 17:26, Heikki Linnakangas wrote: I think that is surprising behavior from the DSA facility. When you make allocations with

Re: ResourceOwner refactoring

2023-11-12 Thread Thomas Munro
On Mon, Nov 13, 2023 at 11:16 AM Heikki Linnakangas wrote: > On 11/11/2023 14:00, Alexander Lakhin wrote: > > 10.11.2023 17:26, Heikki Linnakangas wrote: > >> I think that is surprising behavior from the DSA facility. When you make > >> allocations with dsa_allocate() or just call > >>

Re: ResourceOwner refactoring

2023-11-12 Thread Heikki Linnakangas
On 11/11/2023 14:00, Alexander Lakhin wrote: 10.11.2023 17:26, Heikki Linnakangas wrote: I think that is surprising behavior from the DSA facility. When you make allocations with dsa_allocate() or just call dsa_get_address() on an existing dsa_pointer, you wouldn't expect the current

Re: ResourceOwner refactoring

2023-11-11 Thread Alexander Lakhin
Hello Heikki, 10.11.2023 17:26, Heikki Linnakangas wrote: I think that is surprising behavior from the DSA facility. When you make allocations with dsa_allocate() or just call dsa_get_address() on an existing dsa_pointer, you wouldn't expect the current resource owner to matter. I think

Re: ResourceOwner refactoring

2023-11-10 Thread Andres Freund
Hi, On 2023-11-10 16:26:51 +0200, Heikki Linnakangas wrote: > The quick, straightforward fix is to move the "CurrentResourceOwner = NULL" > line earlier in CommitTransaction, per attached > 0003-Clear-CurrentResourceOwner-earlier-in-CommitTransact.patch. You're not > allowed to use the resource

Re: ResourceOwner refactoring

2023-11-10 Thread Heikki Linnakangas
Thanks for the testing again! On 10/11/2023 11:00, Alexander Lakhin wrote: I could see two failure modes: 2023-11-10 08:42:28.870 UTC [1163274] ERROR:  ResourceOwnerEnlarge called after release started 2023-11-10 08:42:28.870 UTC [1163274] STATEMENT:  drop table t; 2023-11-10 08:42:28.870 UTC

Re: ResourceOwner refactoring

2023-11-10 Thread Alexander Lakhin
Hello Heikki, 09.11.2023 02:48, Heikki Linnakangas wrote: Thanks for the testing! Fixed. ... Thank you for the fix! Please look at one more failure caused be the new implementation of ResourceOwners: numdbs=80 for ((i=1;i<=10;i++)); do echo "ITERATION $i" for ((d=1;d<=$numdbs;d++)); do

Re: ResourceOwner refactoring

2023-11-08 Thread Heikki Linnakangas
On 08/11/2023 20:00, Alexander Lakhin wrote: Please look at a new assertion failure, I've managed to trigger with this script: CREATE TABLE t( i01 int, i02 int, i03 int, i04 int, i05 int, i06 int, i07 int, i08 int, i09 int, i10 int, i11 int, i12 int, i13 int, i14 int, i15 int, i16 int, i17

Re: ResourceOwner refactoring

2023-11-08 Thread Alexander Lakhin
Hello Heikki, 08.11.2023 14:37, Heikki Linnakangas wrote: Fixed these, and pushed. Thanks everyone for reviewing! Please look at a new assertion failure, I've managed to trigger with this script: CREATE TABLE t( i01 int, i02 int, i03 int, i04 int, i05 int, i06 int, i07 int, i08 int, i09

Re: ResourceOwner refactoring

2023-11-08 Thread Heikki Linnakangas
On 07/11/2023 16:00, Alexander Lakhin wrote: 07.11.2023 14:28, Heikki Linnakangas wrote: I feel pretty good about this overall. Barring objections or new cfbot failures, I will commit this in the next few days. A script, that I published in [1], detects several typos in the patches: betwen

Re: ResourceOwner refactoring

2023-11-07 Thread Alexander Lakhin
Hello Heikki, 07.11.2023 14:28, Heikki Linnakangas wrote: I feel pretty good about this overall. Barring objections or new cfbot failures, I will commit this in the next few days. A script, that I published in [1], detects several typos in the patches: betwen ther ReourceOwner

Re: ResourceOwner refactoring

2023-11-07 Thread Heikki Linnakangas
On 25/10/2023 21:07, Andres Freund wrote: It's not too awful to have it be in this order: struct ResourceOwnerData { ResourceOwner parent; /* 0 8 */ ResourceOwner firstchild; /* 8 8 */ ResourceOwner

Re: ResourceOwner refactoring

2023-11-06 Thread Peter Eisentraut
It looks like this patch set needs a bit of surgery to adapt to the LLVM changes in 9dce22033d. The cfbot is reporting compiler warnings about this, and also some crashes, which might also be caused by this. I do like the updated APIs. (Maybe the repeated ".DebugPrint = NULL, /*

Re: ResourceOwner refactoring

2023-10-25 Thread Andres Freund
Hi, On 2023-10-25 15:43:36 +0300, Heikki Linnakangas wrote: > On 10/07/2023 22:14, Andres Freund wrote: > > > /* > > > - * Initially allocated size of a ResourceArray. Must be power of two > > > since > > > - * we'll use (arraysize - 1) as mask for hashing. > > > + * Size of the fixed-size

Re: ResourceOwner refactoring

2023-10-25 Thread Heikki Linnakangas
On 10/07/2023 15:37, Peter Eisentraut wrote: A few suggestions on the API: > +static ResourceOwnerFuncs tupdesc_resowner_funcs = These aren't all "functions", so maybe another word like "info" or "description" would be more appropriate? > + .release_phase =

Re: ResourceOwner refactoring

2023-07-10 Thread Andres Freund
Hi, On 2023-07-10 12:14:46 -0700, Andres Freund wrote: > > /* > > - * Initially allocated size of a ResourceArray. Must be power of two since > > - * we'll use (arraysize - 1) as mask for hashing. > > + * Size of the fixed-size array to hold most-recently remembered resources. > > */ > >

Re: ResourceOwner refactoring

2023-07-10 Thread Andres Freund
Hi, On 2023-05-25 20:14:23 +0300, Heikki Linnakangas wrote: > @@ -2491,9 +2495,6 @@ BufferSync(int flags) > int mask = BM_DIRTY; > WritebackContext wb_context; > > - /* Make sure we can handle the pin inside SyncOneBuffer */ > -

Re: ResourceOwner refactoring

2023-07-10 Thread Peter Eisentraut
A few suggestions on the API: > +static ResourceOwnerFuncs tupdesc_resowner_funcs = These aren't all "functions", so maybe another word like "info" or "description" would be more appropriate? > + .release_phase = RESOURCE_RELEASE_AFTER_LOCKS, > + .release_priority =

Re: ResourceOwner refactoring

2023-07-04 Thread Aleksander Alekseev
Hi, > Thanks, here's a fixed version. (ResourceOwner resource release > callbacks mustn't call ResourceOwnerForget anymore, but AbortBufferIO > was still doing that) I believe v15 is ready to be merged. I suggest we merge it early in the PG17 release cycle. -- Best regards, Aleksander Alekseev

Re: ResourceOwner refactoring

2023-05-23 Thread Aleksander Alekseev
Hi, > [...] > A-ha, it's because I didn't add the new test_resowner directory to the > src/test/modules/meson.build file. Fixed. > > Thanks for the review! You probably already noticed, but for the record: cfbot seems to be extremely unhappy with the patch. -- Best regards, Aleksander Alekseev

Re: ResourceOwner refactoring

2023-03-30 Thread Aleksander Alekseev
Hi, > This patch, although moderately complicated, was moved between several > commitfests. I think it would be great if it made it to PG16. I'm > inclined to change the status of the patchset to RfC in a bit, unless > anyone has a second opinion. > I added a test module in

Re: ResourceOwner refactoring

2023-03-22 Thread Aleksander Alekseev
Hi, I noticed that the patchset didn't make much progress since February and decided to give it another round of code review. > [...]. But in general, end-of-transaction activities should be kept > simple, especially between the release phases, so I feel that having to > remember extra resources

Re: ResourceOwner refactoring

2022-11-01 Thread Andres Freund
Hi, On 2022-11-01 12:39:39 +0200, Heikki Linnakangas wrote: > > 1) As far as I can tell, the way ResourceOwnerReleaseAll() now works seems > > to > > assume that within a phase the reset order does not matter. I don't > > think > > that's a good assumption. I e.g. have a patch to

Re: ResourceOwner refactoring

2022-11-01 Thread Robert Haas
On Tue, Nov 1, 2022 at 6:39 AM Heikki Linnakangas wrote: > However, I feel that trying to enforce a particular order moves the > goalposts. If we need that, let's add it as a separate patch later. I don't really see it that way, because with the current implementation, we do all resources of a

Re: ResourceOwner refactoring

2022-11-01 Thread Heikki Linnakangas
On 01/11/2022 02:15, Andres Freund wrote: On 2022-10-31 10:51:36 +0100, Heikki Linnakangas wrote: These are functions where quite a lot of things happen between the ResourceOwnerEnlarge and ResourceOwnerRemember calls. It's important that there are no unrelated ResourceOwnerRemember() calls in

Re: ResourceOwner refactoring

2022-10-31 Thread Andres Freund
Hi, On 2022-10-31 10:51:36 +0100, Heikki Linnakangas wrote: > These are functions where quite a lot of things happen between the > ResourceOwnerEnlarge and ResourceOwnerRemember calls. It's important that > there are no unrelated ResourceOwnerRemember() calls in the code in > between, otherwise

Re: ResourceOwner refactoring

2022-10-31 Thread Aleksander Alekseev
Hi hackers, > > Rebased version attached. Given that Aleksander marked this as Ready for > > Committer earlier, I'll add this to the next commitfest in that state, > > and will commit in the next few days, barring any new objections. > > Thanks for resurrecting this patch. Additionally I decided

Re: ResourceOwner refactoring

2022-10-31 Thread Aleksander Alekseev
Hi Heikki, > Rebased version attached. Given that Aleksander marked this as Ready for > Committer earlier, I'll add this to the next commitfest in that state, > and will commit in the next few days, barring any new objections. Thanks for resurrecting this patch. While taking a fresh look at the

Re: ResourceOwner refactoring

2022-10-31 Thread Heikki Linnakangas
On 12/01/2022 07:57, Julien Rouhaud wrote: On Fri, Nov 26, 2021 at 8:41 PM Aleksander Alekseev wrote: The patchset is in a good shape. I'm changing the status to "Ready for Committer". The 2nd patch doesn't apply anymore due to a conflict on resowner_private.h:

Re: ResourceOwner refactoring

2022-01-11 Thread Julien Rouhaud
Hi, On Fri, Nov 26, 2021 at 8:41 PM Aleksander Alekseev wrote: > > > I will submit the actual code review in the follow-up email > > The patchset is in a good shape. I'm changing the status to "Ready for > Committer". The 2nd patch doesn't apply anymore due to a conflict on resowner_private.h:

Re: ResourceOwner refactoring

2021-11-26 Thread Aleksander Alekseev
Hi Heikki, > I will submit the actual code review in the follow-up email The patchset is in a good shape. I'm changing the status to "Ready for Committer". -- Best regards, Aleksander Alekseev

Re: ResourceOwner refactoring

2021-11-23 Thread Aleksander Alekseev
Hi Heikki, > Attached is a newly rebased version. It includes your tweaks, and a few > more comment and indentation tweaks. v10-0002 rotted a little so I rebased it. The new patch set passed `make installcheck-world` locally, but let's see if cfbot has a second opinion. I'm a bit busy with

Re: ResourceOwner refactoring

2021-10-18 Thread Heikki Linnakangas
On 08/09/2021 13:19, Aleksander Alekseev wrote: Hi Heikki, Yeah, needed some manual fixing, but here you go. Thanks for working on this! v8-0002 didn't apply to the current master, so I rebased it. See attached v9-* patches. I also included v9-0004 with some minor tweaks from me. I have

Re: ResourceOwner refactoring

2021-10-01 Thread Michael Paquier
On Wed, Sep 08, 2021 at 01:19:19PM +0300, Aleksander Alekseev wrote: > Thanks for working on this! The latest patch does not apply anymore, and has been waiting on author for a couple of weeks now, so switched as RwF for now. -- Michael signature.asc Description: PGP signature

Re: ResourceOwner refactoring

2021-09-08 Thread Aleksander Alekseev
Hi Heikki, > Yeah, needed some manual fixing, but here you go. Thanks for working on this! v8-0002 didn't apply to the current master, so I rebased it. See attached v9-* patches. I also included v9-0004 with some minor tweaks from me. I have several notes regarding the code. 1. Not sure if I

Re: ResourceOwner refactoring

2021-07-14 Thread Zhihong Yu
On Wed, Jul 14, 2021 at 8:26 AM Heikki Linnakangas wrote: > Thanks for having a look! > > On 14/07/2021 18:18, Zhihong Yu wrote: > > For the loop over the hash: > > > > + for (int idx = 0; idx < capacity; idx++) > > { > > - if (olditemsarr[i] != resarr->invalidval) > > -

Re: ResourceOwner refactoring

2021-07-14 Thread Heikki Linnakangas
Thanks for having a look! On 14/07/2021 18:18, Zhihong Yu wrote: For the loop over the hash: +       for (int idx = 0; idx < capacity; idx++)         { -           if (olditemsarr[i] != resarr->invalidval) -               ResourceArrayAdd(resarr, olditemsarr[i]); +           while

Re: ResourceOwner refactoring

2021-07-14 Thread Zhihong Yu
On Wed, Jul 14, 2021 at 7:40 AM Heikki Linnakangas wrote: > On 14/07/2021 17:07, Alvaro Herrera wrote: > > On 2021-Jul-14, vignesh C wrote: > > > >> On Tue, Mar 9, 2021 at 6:10 PM Heikki Linnakangas > wrote: > > > >>> Here you go. > >> > >> The patch does not apply on Head anymore, could you

Re: ResourceOwner refactoring

2021-07-14 Thread Heikki Linnakangas
On 14/07/2021 17:07, Alvaro Herrera wrote: On 2021-Jul-14, vignesh C wrote: On Tue, Mar 9, 2021 at 6:10 PM Heikki Linnakangas wrote: Here you go. The patch does not apply on Head anymore, could you rebase and post a patch. I'm changing the status to "Waiting for Author". Support for

Re: ResourceOwner refactoring

2021-07-14 Thread Alvaro Herrera
On 2021-Jul-14, vignesh C wrote: > On Tue, Mar 9, 2021 at 6:10 PM Heikki Linnakangas wrote: > > Here you go. > > The patch does not apply on Head anymore, could you rebase and post a > patch. I'm changing the status to "Waiting for Author". Support for hmac was added by e6bdfd9700eb so the

Re: ResourceOwner refactoring

2021-07-14 Thread vignesh C
On Tue, Mar 9, 2021 at 6:10 PM Heikki Linnakangas wrote: > > On 08/03/2021 18:47, Ibrar Ahmed wrote: > > The patchset does not apply successfully, there are some hunk failures. > > > > http://cfbot.cputube.org/patch_32_2834.log > > > > > >

Re: ResourceOwner refactoring

2021-03-09 Thread Heikki Linnakangas
On 08/03/2021 18:47, Ibrar Ahmed wrote: The patchset does not apply successfully, there are some hunk failures. http://cfbot.cputube.org/patch_32_2834.log v6-0002-Make-resowners-more-easily-extensible.patch 1 out of 6 hunks FAILED -- saving

Re: ResourceOwner refactoring

2021-03-08 Thread Ibrar Ahmed
On Mon, Jan 25, 2021 at 10:15 PM Robert Haas wrote: > On Thu, Jan 21, 2021 at 5:14 AM Heikki Linnakangas > wrote: > > Here you can see that as numsnaps increases, the test becomes slower, > > but then it becomes faster again at 64-66, when it switches to the hash > > table. So 64 seems too

Re: ResourceOwner refactoring

2021-01-25 Thread Robert Haas
On Thu, Jan 21, 2021 at 5:14 AM Heikki Linnakangas wrote: > Here you can see that as numsnaps increases, the test becomes slower, > but then it becomes faster again at 64-66, when it switches to the hash > table. So 64 seems too much. 32 seems to be the sweet spot here, that's > where scanning

Re: ResourceOwner refactoring

2021-01-21 Thread Heikki Linnakangas
On 21/01/2021 06:14, Michael Paquier wrote: On Thu, Jan 21, 2021 at 12:11:37AM +0200, Heikki Linnakangas wrote: Summary: In the the worst scenario, the patched version is still 24% slower than unpatched. But many other scenarios are now faster with the patch. Is there a reason explaining the

Re: ResourceOwner refactoring

2021-01-20 Thread Michael Paquier
On Thu, Jan 21, 2021 at 12:11:37AM +0200, Heikki Linnakangas wrote: > Summary: In the the worst scenario, the patched version is still 24% slower > than unpatched. But many other scenarios are now faster with the patch. Is there a reason explaining the sudden drop for numsnaps within the [10,60]

RE: ResourceOwner refactoring

2021-01-20 Thread kuroda.hay...@fujitsu.com
Dear Heikki, I tested in the same situation, and I confirmed that almost same results are returned. (results are at the end of the e-mail) You think that these results are acceptable because resowners own many resources(more than 64) in general and it's mainly faster in such a situation, isn't

Re: ResourceOwner refactoring

2021-01-20 Thread Heikki Linnakangas
On 19/01/2021 11:09, Heikki Linnakangas wrote: On 18/01/2021 18:11, Robert Haas wrote: On Mon, Jan 18, 2021 at 11:11 AM Robert Haas wrote: On Mon, Jan 18, 2021 at 10:19 AM Heikki Linnakangas wrote: On 18/01/2021 16:34, Alvaro Herrera wrote: So according to your performance benchmark, we're

Re: ResourceOwner refactoring

2021-01-19 Thread Heikki Linnakangas
On 18/01/2021 16:34, Alvaro Herrera wrote: On 2021-Jan-18, Heikki Linnakangas wrote: +static ResourceOwnerFuncs jit_funcs = +{ + /* relcache references */ + .name = "LLVM JIT context", + .phase = RESOURCE_RELEASE_BEFORE_LOCKS, + .ReleaseResource =

Re: ResourceOwner refactoring

2021-01-19 Thread Heikki Linnakangas
On 18/01/2021 18:11, Robert Haas wrote: On Mon, Jan 18, 2021 at 11:11 AM Robert Haas wrote: On Mon, Jan 18, 2021 at 10:19 AM Heikki Linnakangas wrote: On 18/01/2021 16:34, Alvaro Herrera wrote: So according to your performance benchmark, we're willing to accept a 30% performance loss on an

Re: ResourceOwner refactoring

2021-01-18 Thread Michael Paquier
On Mon, Jan 18, 2021 at 02:22:33PM +0200, Heikki Linnakangas wrote: > diff --git a/src/common/cryptohash_openssl.c b/src/common/cryptohash_openssl.c > index 551ec392b60..642e72d8c0f 100644 > --- a/src/common/cryptohash_openssl.c > +++ b/src/common/cryptohash_openssl.c [...] > +/* ResourceOwner

Re: ResourceOwner refactoring

2021-01-18 Thread Robert Haas
On Mon, Jan 18, 2021 at 11:11 AM Robert Haas wrote: > On Mon, Jan 18, 2021 at 10:19 AM Heikki Linnakangas wrote: > > On 18/01/2021 16:34, Alvaro Herrera wrote: > > > So according to your performance benchmark, we're willing to accept a > > > 30% performance loss on an allegedly common operation

Re: ResourceOwner refactoring

2021-01-18 Thread Robert Haas
On Mon, Jan 18, 2021 at 10:19 AM Heikki Linnakangas wrote: > > On 18/01/2021 16:34, Alvaro Herrera wrote: > > So according to your performance benchmark, we're willing to accept a > > 30% performance loss on an allegedly common operation -- numkeep=0 > > numsnaps=10 becomes 49.8ns from 37.6ns.

Re: ResourceOwner refactoring

2021-01-18 Thread Heikki Linnakangas
On 18/01/2021 16:34, Alvaro Herrera wrote: So according to your performance benchmark, we're willing to accept a 30% performance loss on an allegedly common operation -- numkeep=0 numsnaps=10 becomes 49.8ns from 37.6ns. That seems a bit shocking. Maybe you can claim that these operations aren't

Re: ResourceOwner refactoring

2021-01-18 Thread Alvaro Herrera
On 2021-Jan-18, Heikki Linnakangas wrote: > +static ResourceOwnerFuncs jit_funcs = > +{ > + /* relcache references */ > + .name = "LLVM JIT context", > + .phase = RESOURCE_RELEASE_BEFORE_LOCKS, > + .ReleaseResource = ResOwnerReleaseJitContext, > + .PrintLeakWarning =

Re: ResourceOwner refactoring

2021-01-18 Thread Heikki Linnakangas
On 18/01/2021 09:49, kuroda.hay...@fujitsu.com wrote: Dear Heikki, I apologize for sending again. I will check another ResourceOwnerEnlarge() if I have a time. I checked all ResourceOwnerEnlarge() types after applying patch 0001. (searched by "grep -rI ResourceOwnerEnlarge") No problem was

RE: ResourceOwner refactoring

2021-01-17 Thread kuroda.hay...@fujitsu.com
Dear Heikki, I apologize for sending again. > I will check another ResourceOwnerEnlarge() if I have a time. I checked all ResourceOwnerEnlarge() types after applying patch 0001. (searched by "grep -rI ResourceOwnerEnlarge") No problem was found. Hayato Kuroda FUJITSU LIMITED

RE: ResourceOwner refactoring

2021-01-15 Thread kuroda.hay...@fujitsu.com
Dear Heikki, > Hmm. ResOwnerReleaseTupleDesc() does exist, those functions are needed > for the callbacks. I think you meant the wrappers around > ResourceOwnerRemember and ResourceOwnerForget, like > ResourceOwnerRememberCatCacheRef(). I admit those are not fully > consistent: I didn't

Re: ResourceOwner refactoring

2021-01-14 Thread Heikki Linnakangas
On 14/01/2021 12:15, kuroda.hay...@fujitsu.com wrote: I put some comments. Thanks for the review! Throughout, some components don’t have helper functions. (e.g. catcache has ResOwnerReleaseCatCache, but tupdesc doesn't.) I think it should be unified. Hmm. ResOwnerReleaseTupleDesc() does

RE: ResourceOwner refactoring

2021-01-14 Thread kuroda.hay...@fujitsu.com
Hi, I put some comments. Throughout, some components don’t have helper functions. (e.g. catcache has ResOwnerReleaseCatCache, but tupdesc doesn't.) I think it should be unified. [catcache.c] > +/* support for catcache refcount management */ > +static inline void >

RE: ResourceOwner refactoring

2021-01-12 Thread kuroda.hay...@fujitsu.com
Dear Heikki, Thank you for rebasing it, I confirmed it can be applied. I will check the source. Now I put the very elementary comment. ResourceOwnerEnlarge(), ResourceOwnerRemember(), and ResourceOwnerForget() are exported routines. They should put below L418. Best regards, Hayato Kuroda

Re: ResourceOwner refactoring

2021-01-12 Thread Michael Paquier
On Wed, Jan 13, 2021 at 09:18:57AM +0200, Heikki Linnakangas wrote: > --- a/src/common/cryptohash_openssl.c > +++ b/src/common/cryptohash_openssl.c > +static ResourceOwnerFuncs cryptohash_funcs = > +{ > + /* relcache references */ > + .name = "LLVM JIT context", > + .phase =

Re: ResourceOwner refactoring

2021-01-12 Thread Heikki Linnakangas
On 13/01/2021 03:55, kuroda.hay...@fujitsu.com wrote: Dear Heikki, I'm also interested in this patch, but it cannot be applied to the current HEAD... $ git apply ~/v2-0001-Make-resowners-more-easily-extensible.patch error: patch failed: src/common/cryptohash_openssl.c:57 error:

RE: ResourceOwner refactoring

2021-01-12 Thread kuroda.hay...@fujitsu.com
Dear Heikki, I'm also interested in this patch, but it cannot be applied to the current HEAD... $ git apply ~/v2-0001-Make-resowners-more-easily-extensible.patch error: patch failed: src/common/cryptohash_openssl.c:57 error: src/common/cryptohash_openssl.c: patch does not apply error: patch

Re: ResourceOwner refactoring

2020-12-16 Thread Heikki Linnakangas
On 26/11/2020 10:52, Kyotaro Horiguchi wrote: + for (int i = 0; i < owner->narr; i++) { + if (owner->arr[i].kind->phase == phase) + { + Datum value = owner->arr[i].item; +

Re: ResourceOwner refactoring

2020-11-26 Thread Kyotaro Horiguchi
At Thu, 19 Nov 2020 17:27:18 +0800, Julien Rouhaud wrote in > On Thu, Nov 19, 2020 at 10:16 AM Michael Paquier 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

Re: ResourceOwner refactoring

2020-11-25 Thread Michael Paquier
On Thu, Nov 19, 2020 at 06:40:19PM +0800, Craig Ringer wrote: > [off-list for now] Looks like you have missed something here. -- Michael signature.asc Description: PGP signature

Re: ResourceOwner refactoring

2020-11-19 Thread Craig Ringer
[off-list for now] On Tue, Nov 17, 2020 at 10:21 PM 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

Re: ResourceOwner refactoring

2020-11-19 Thread Julien Rouhaud
On Thu, Nov 19, 2020 at 10:16 AM Michael Paquier 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

Re: ResourceOwner refactoring

2020-11-18 Thread Michael Paquier
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

Re: ResourceOwner refactoring

2020-11-18 Thread Heikki Linnakangas
On 18/11/2020 10:06, Michael Paquier wrote: On Tue, Nov 17, 2020 at 04:21:29PM +0200, Heikki Linnakangas wrote: Attached patch refactors the ResourceOwner internals to do that. + * Size of the small fixed-size array to hold most-recently remembered resources. */ -#define

Re: ResourceOwner refactoring

2020-11-18 Thread Michael Paquier
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