Re: ResourceOwner refactoring - Mailing list pgsql-hackers

From Zhihong Yu
Subject Re: ResourceOwner refactoring
Date
Msg-id CALNJ-vTh=O0O0ENOMC4uGd1gbzcnHaXTi8W4W=qri5Na3-LMKw@mail.gmail.com
Whole thread Raw
In response to Re: ResourceOwner refactoring  (Heikki Linnakangas <hlinnaka@iki.fi>)
Responses Re: ResourceOwner refactoring  (Aleksander Alekseev <aleksander@timescale.com>)
List pgsql-hackers


On Wed, Jul 14, 2021 at 8:26 AM Heikki Linnakangas <hlinnaka@iki.fi> 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)
> -               ResourceArrayAdd(resarr, olditemsarr[i]);
> +           while (owner->hash[idx].kind != NULL &&
> +                  owner->hash[idx].kind->phase == phase)
> ...
> +   } while (capacity != owner->capacity);
>
> Since the phase variable doesn't seem to change for the while loop, I
> wonder what benefit the while loop has (since the release is governed by
> phase).

Hmm, the phase variable doesn't change, but could the element at
'owner->hash[idx]' change? I'm not sure about that. The loop is supposed
to handle the case that the hash table grows; could that replace the
element at 'owner->hash[idx]' with something else, with different phase?
The check is very cheap, so I'm inclined to keep it to be sure.

- Heikki
Hi,
Agreed that ```owner->hash[idx].kind->phase == phase``` can be kept.

I just wonder if we should put limit on the number of iterations for the while loop.
Maybe add a bool variable indicating that kind->ReleaseResource(value) is called in the inner loop.
If there is no releasing when the inner loop finishes, we can come out of the while loop.

Cheers

pgsql-hackers by date:

Previous
From: Mark Dilger
Date:
Subject: Re: Extending amcheck to check toast size and compression
Next
From: vignesh C
Date:
Subject: Re: [PATCH] Allow multiple recursive self-references