Message-ID: <818772a0-bb59-4107-8f1d-749e8b498b9a@iki.fi>
Date: Tue, 6 Jan 2026 17:17:13 +0200
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: Refactor PROCLOCK hash table into partitioned list allocator
To: Matthias van de Meent <boekewurm+postgres@gmail.com>
Cc: Andrey Borodin <x4mmm@yandex-team.ru>,
 PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
References: <271502B5-C950-4428-AF69-9C6D62EF5F14@yandex-team.ru>
 <3d7e7d38-b3c1-4adf-8cb0-309dba08dba9@iki.fi>
 <CAEze2Wh5Qwit7WZhcJyYRWxVM_M-bf1VBkHqqp9nCJ6rfHTWMQ@mail.gmail.com>
Content-Language: en-US
From: Heikki Linnakangas <hlinnaka@iki.fi>
In-Reply-To: <CAEze2Wh5Qwit7WZhcJyYRWxVM_M-bf1VBkHqqp9nCJ6rfHTWMQ@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Archived-At: <https://www.postgresql.org/message-id/818772a0-bb59-4107-8f1d-749e8b498b9a%40iki.fi>
Precedence: bulk

On 06/01/2026 16:58, Matthias van de Meent wrote:
> On Tue, 6 Jan 2026 at 15:24, Heikki Linnakangas <hlinnaka@iki.fi> wrote:
>>
>> On 30/12/2025 14:37, Andrey Borodin wrote:
>>> Hi hackers,
>>>
>>> Following up on the Discord discussion about the PROCLOCK hash table being
>>> a "weird allocator" that we never actually use for lookups - I took a stab at
>>> replacing it with a simpler partitioned free list approach as was suggested.
>>> I was doing this mostly to educate myself on Lock Manager internals.
>>>
>>> The current implementation uses LockMethodProcLockHash purely as an allocator.
>>> We never do hash lookups by key; we only allocate entries, link them to the lock's
>>> procLocks list, and free them later. Using a full hash table for this adds
>>> unnecessary complexity and maybe even overhead (I did not measure this).
>>>
>>> The attached patch replaces this with:
>>> - ProcLockArray: A fixed-size array of all PROCLOCK structs (allocated at startup)
>>> - ProcLockFreeList: Partitioned free lists, one per lock partition to reduce contention
>>> - ProcLockAlloc/Free: Simple push/pop operations on the free lists
>>> - PROCLOCK lookup: Linear traversal of lock->procLocks (see LockRefindAndRelease()
>>>     and FastPathGetRelationLockEntry())
>>>
>>> The last point bothers me most. It seems like this traversals are expected to be short.
>>> But I'm not 100% sure.
>>
>> Hmm, yeah the last point contradicts the premise that the hash table is
>> used purely as an allocator. It *is* used for lookups, and you're
>> replacing them with linear scans. That doesn't seem like an improvement.
> 
> There are 2 types of PROCLOCK lookup used in LockRefindAndRelease and
> FastPathGetRelationLockEntry:
> - An active backend's PROCLOCK entries (in both LRAR and FPGRLE).
> - Prepared transaction's PROCLOCK entries (only in LRAR, called from
> lock_twophase_postcommit).

There are also lookups in SetupLockInTable and in LockRelease.

> For the backend's PROCLOCK entry lookup, we can use a backend-local
> hash table, which only keeps track of where this backend's entries
> are.

Hmm, good point. In fact we already have that: there's a pointer to the 
current process's PROCLOCK entry in LOCALLOCK already. Can we use that 
to avoid the linear scans? There's this LockAcquireExtended:

> 	/*
> 	 * Find or create lock and proclock entries with this tag
> 	 *
> 	 * Note: if the locallock object already existed, it might have a pointer
> 	 * to the lock already ... but we should not assume that that pointer is
> 	 * valid, since a lock object with zero hold and request counts can go
> 	 * away anytime.  So we have to use SetupLockInTable() to recompute the
> 	 * lock and proclock pointers, even if they're already set.
> 	 */
> 	proclock = SetupLockInTable(lockMethodTable, MyProc, locktag,
> 								hashcode, lockmode);

So that comment suggests that the 'proclock' pointer cannot be trusted 
here. I don't remember how all this works, so I'm not sure if that is a 
show-stopper or if we could somehow leverage the 'proclock' pointer in 
LOCALLOCK anyway.

- Heikki