Re: [COMMITTERS] pgsql: Reduce the number of semaphores used under --disable-spinlocks.

On Wed, Jun 18, 2014 at 3:32 PM, Andres Freund <andres@2ndquadrant.com> wrote:
> Hi,
>
> On 2014-01-08 23:58:16 +0000, Robert Haas wrote:
>> Reduce the number of semaphores used under --disable-spinlocks.
>>
>> Instead of allocating a semaphore from the operating system for every
>> spinlock, allocate a fixed number of semaphores (by default, 1024)
>> from the operating system and multiplex all the spinlocks that get
>> created onto them.  This could self-deadlock if a process attempted
>> to acquire more than one spinlock at a time, but since processes
>> aren't supposed to execute anything other than short stretches of
>> straight-line code while holding a spinlock, that shouldn't happen.
>>
>> One motivation for this change is that, with the introduction of
>> dynamic shared memory, it may be desirable to create spinlocks that
>> last for less than the lifetime of the server.  Without this change,
>> attempting to use such facilities under --disable-spinlocks would
>> quickly exhaust any supply of available semaphores.  Quite apart
>> from that, it's desirable to contain the quantity of semaphores
>> needed to run the server simply on convenience grounds, since using
>> too many may make it harder to get PostgreSQL running on a new
>> platform, which is mostly the point of --disable-spinlocks in the
>> first place.
>
> I'm looking at the way you did this in the context of the atomics
> patch. Won't:
> s_init_lock_sema(volatile slock_t *lock)
> {
>         static int      counter = 0;
>
>         *lock = (++counter) % NUM_SPINLOCK_SEMAPHORES;
> }
>
> lead to bad results if spinlocks are intialized after startup?

Why?

> Essentially mapping new spinlocks to the same semaphore?

Yeah, but so what?  If we're mapping a bajillion spinlocks to the same
semaphore already, what's a few more?

> That's a
> restriction I can live with, especially as this is only for super old
> platforms. But it might be worth mentioning somewhere?

Dunno.  What restriction?

> I've essentially reeimplemented that kind of logic in the atomics
> patch. Looking to get rid of the duplication... There I used something
> like
> slot = ((uintptr_t) addr_of_atomic >> (sizeof(void*) + 5)) % NUM_LOCKS
> but I think your method is actually better because it allows to place
> spinlocks/atomics to be placed in dsm segments placed at different
> location in individual processes.

Right.

> My current plan to get rid of the duplication is to simply embed the
> spinlock inside the atomic variable instead of having a separate array
> of spinlocks protecting atomic variables.

Doesn't sound crazy at first glance.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company