Re: [PATCH] Refactoring of LWLock tranches - Mailing list pgsql-hackers

From Ildus Kurbangaliev
Subject Re: [PATCH] Refactoring of LWLock tranches
Date
Msg-id 20150907113122.03ff9070@iw
Whole thread Raw
In response to Re: [PATCH] Refactoring of LWLock tranches  ("andres@anarazel.de" <andres@anarazel.de>)
Responses Re: [PATCH] Refactoring of LWLock tranches  (Ildus Kurbangaliev <i.kurbangaliev@postgrespro.ru>)
List pgsql-hackers
On Sun, 6 Sep 2015 23:18:02 +0200
"andres@anarazel.de" <andres@anarazel.de> wrote:

> On 2015-09-06 22:57:04 +0300, Ildus Kurbangaliev wrote:
> > Ok, I've kept only one tranche for individual LWLocks
> 
> But you've added the lock names as a statically sized array to all
> tranches? Why not just a pointer to an array that's either NULL ('not
> individualy named locks') or appropriately sized?

A tranche contains only the tranche name. Yes, it's statically sized,
because we need to know an exact space in shared memory for it. This 
name is enough to describe all LWLocks in that tranche (except main
tranche), because they have one type (for example BufferMgrLocks). 
In main tranche this field contains 'main' (like now) and 
an additional array is used for determining a name for LWLock. If you
suggest keep a pointer to this array in main tranche (and NULL for
others) then I have no objections to do that.

So generally the code is similar to code that we have in Postgres now
except the tranches located in shared memory and created by backends.

> 
> > > I don't really like the tranche model as in the patch right now.
> > > I'd rather have in a way that we have one tranch for all the
> > > individual lwlocks, where the tranche points to an array of names
> > > alongside the tranche's name. And then for the others we just
> > > supply the tranche name, but leave the name array empty, whereas
> > > a name can be generated.
> > 
> > Maybe I don't understand something here, but why add extra field to
> > all tranches if we need only one array (especially the array for
> > individual LWLocks).
> 
> It's cheap to add an optional pointer field to an individual
> tranche. It'll be far less cheap to extend the max number of
> tranches. But it's also just ugly to to use a tranche per lock -
> they're intended to describe 'runs' of locks.

The current patch exactly does that - contains 'runs' of locks.

----
Ildus Kurbangaliev
Postgres Professional: http://www.postgrespro.com
<http://www.postgrespro.com/> The Russian Postgres Company



pgsql-hackers by date:

Previous
From: Etsuro Fujita
Date:
Subject: Re: Foreign join pushdown vs EvalPlanQual
Next
From: Kyotaro HORIGUCHI
Date:
Subject: Re: Foreign join pushdown vs EvalPlanQual