Re: ResourceOwner refactoring - Mailing list pgsql-hackers

From Aleksander Alekseev
Subject Re: ResourceOwner refactoring
Date
Msg-id CAJ7c6TP8v=2Ym5S1C4YenaCMgwu5FrFTGi40Nn5TyDS4ig-gRA@mail.gmail.com
Whole thread Raw
In response to Re: ResourceOwner refactoring  (Zhihong Yu <zyu@yugabyte.com>)
Responses Re: ResourceOwner refactoring  (Michael Paquier <michael@paquier.xyz>)
Re: ResourceOwner refactoring  (Heikki Linnakangas <hlinnaka@iki.fi>)
List pgsql-hackers
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 understand this code of ResourceOwnerReleaseAll():
```
    /* First handle all the entries in the array. */
    do
    {
        found = false;
        for (int i = 0; i < owner->narr; i++)
        {
            if (owner->arr[i].kind->phase == phase)
            {
                Datum        value = owner->arr[i].item;
                ResourceOwnerFuncs *kind = owner->arr[i].kind;

                if (printLeakWarnings)
                    kind->PrintLeakWarning(value);
                kind->ReleaseResource(value);
                found = true;
            }
        }

        /*
         * If any resources were released, check again because some of the
         * elements might have been moved by the callbacks. We don't want to
         * miss them.
         */
    } while (found && owner->narr > 0);
```

Shouldn't we mark the resource as released and/or decrease narr and/or
save the last processed i? Why this will not call ReleaseResource()
endlessly on the same resource (array item)? Same question for the
following code that iterates over the hash table.

2. Just an idea/observation. It's possible that the performance of
ResourceOwnerEnlarge() can be slightly improved. Since the size of the
hash table is always a power of 2 and the code always doubles the size
of the hash table, (idx & mask) value will get one extra bit, which
can be 0 or 1. If it's 0, the value is already in its place,
otherwise, it should be moved on the known distance. In other words,
it's possible to copy `oldhash` to `newhash` and then move only half
of the items. I don't claim that this code necessarily should be
faster, or that this should be checked in the scope of this work.

-- 
Best regards,
Aleksander Alekseev

Attachment

pgsql-hackers by date:

Previous
From: "wangsh.fnst@fujitsu.com"
Date:
Subject: drop tablespace failed when location contains .. on win32
Next
From: Dilip Kumar
Date:
Subject: Re: Gather performance analysis