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

From Amit Kapila
Subject Re: [PATCH] Refactoring of LWLock tranches
Date
Msg-id CAA4eK1JzWH17tMP8e-kTS_zNyLpX2ROeLurmgqEu20g4ZX+sJA@mail.gmail.com
Whole thread Raw
In response to Re: [PATCH] Refactoring of LWLock tranches  (Robert Haas <robertmhaas@gmail.com>)
Responses Re: [PATCH] Refactoring of LWLock tranches
List pgsql-hackers
On Wed, Feb 10, 2016 at 8:51 PM, Robert Haas <robertmhaas@gmail.com> wrote:
>
> On Wed, Feb 10, 2016 at 1:32 AM, Amit Kapila <amit.kapila16@gmail.com> wrote:
> > The reason for using centralized way is that we need to request
> > named tranches before initialization of shared memory and as far as
> > I can see, currently there is no way in the subsystems where they can
> > issue such a request, so one possibility is that we introduce new API's
> > like InitBufferLWLocks(), InitLmgrLWLocks(), InitPredicateLWLocks()
> > in respective subsystem and call them in
> > CreateSharedMemoryAndSemaphores() before shared memory
> > initialization. Does by doing that way addresses your concern?
>
> Well, if we're going to have new functions like that, I think the
> place to call them from is PostmasterMain() just before
> process_shared_preload_libraries(). After all, if extensions were
> requesting tranches, they'd do it from
> process_shared_preload_libraries(), so it seems like the right place.
>

Initially I also thought like that, but on further analysis I found that
we also need to request the tranches from InitCommunication() as
that gets called from initdb path.

> However, since the number of locks we need for each of these
> subsystems is fixed at compile time, it seems a bit of a shame to have
> to do something about them at runtime.  I wonder if we should just
> hard-code this in CreateLWLocks() instead of trying to use the
> named-tranche facility.  That is, where that function does this:
>
>     MainLWLockTranche.name = "main";
>     MainLWLockTranche.array_base = MainLWLockArray;
>     MainLWLockTranche.array_stride = sizeof(LWLockPadded);
>     LWLockRegisterTranche(LWTRANCHE_MAIN, &MainLWLockTranche);
>
> ...register four tranches instead.  And where it does this:
>
>         /* Initialize all fixed LWLocks in main array */
>         for (id = 0, lock = MainLWLockArray; id < numLocks; id++, lock++)
>             LWLockInitialize(&lock->lock, LWTRANCHE_MAIN);
>
> ...have four loops instead, each initializing with a different tranche
> ID.  Then the current method of computing the location of those locks
> would still work just fine; the code changes would be a lot more
> isolated, and we wouldn't have to do runtime save-and-restore of more
> variables on Windows.
>

Sounds sensible, however after changes, CreateLWLocks() started
looking unreadable, so moved initialization and registration of tranches
to separate functions.

Increased number of tranches allocated in LWLockTrancheArray, as
now the LWTRANCHE_FIRST_USER_DEFINED is already greater
than 16.


With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
Attachment

pgsql-hackers by date:

Previous
From: Artur Zakirov
Date:
Subject: Re: Mac OS: invalid byte sequence for encoding "UTF8"
Next
From: Artur Zakirov
Date:
Subject: Re: Mac OS: invalid byte sequence for encoding "UTF8"