Re: ResourceOwner refactoring

From: Aleksander Alekseev <aleksander(at)timescale(dot)com>
To: "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Cc: Heikki Linnakangas <hlinnaka(at)iki(dot)fi>
Subject: Re: ResourceOwner refactoring
Date: 2021-09-08 10:19:19
Message-ID: CAJ7c6TP8v=2Ym5S1C4YenaCMgwu5FrFTGi40Nn5TyDS4ig-gRA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: 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 Content-Type Size
v9-0003-Optimize-hash-function.patch application/octet-stream 2.3 KB
v9-0001-Move-a-few-ResourceOwnerEnlarge-calls-for-safety.patch application/octet-stream 8.6 KB
v9-0002-Make-resowners-more-easily-extensible.patch application/octet-stream 90.1 KB
v9-0004-Minor-tweaks.patch application/octet-stream 1.4 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Dilip Kumar 2021-09-08 11:11:54 Re: Gather performance analysis
Previous Message wangsh.fnst@fujitsu.com 2021-09-08 10:16:46 drop tablespace failed when location contains .. on win32