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

From Amit Kapila
Subject Re: [PATCH] Refactoring of LWLock tranches
Date
Msg-id CAA4eK1LQKS4hr1i5yOXL9F_D37eSdrUg_1hsLjSG7uHSptutDw@mail.gmail.com
Whole thread Raw
In response to Re: [PATCH] Refactoring of LWLock tranches  (Amit Kapila <amit.kapila16@gmail.com>)
Responses Re: [PATCH] Refactoring of LWLock tranches  (Alvaro Herrera <alvherre@2ndquadrant.com>)
Re: [PATCH] Refactoring of LWLock tranches  (Robert Haas <robertmhaas@gmail.com>)
List pgsql-hackers
On Mon, Dec 28, 2015 at 4:47 PM, Amit Kapila <amit.kapila16@gmail.com> wrote:
On Thu, Dec 24, 2015 at 5:50 PM, Ildus Kurbangaliev <i.kurbangaliev@postgrespro.ru> wrote:
> >
>
> There is a patch that moves backend LWLocks into PGPROC and to a
> separate tranche.
>

1.
@@ -437,6 +440,13 @@ InitProcessPhase2(void)
 {
  Assert(MyProc != NULL);
 
+ /* Register and initialize fields of ProcLWLockTranche */
+ ProcLWLockTranche.name = "proc";
+ ProcLWLockTranche.array_base = (char *) (ProcGlobal->allProcs) +
+ offsetof(PGPROC, backendLock);
+ ProcLWLockTranche.array_stride = sizeof(PGPROC);
+ LWLockRegisterTranche(LWTRANCHE_PROC, &ProcLWLockTranche);
+

I think this will not work for Auxilary processes as they won't
call InitProcessPhase2().  It is better to initialize it in
InitProcGlobal() and then propagate it to backends for EXEC_BACKEND
cases as we do for ProcStructLock, AuxiliaryProcs.


That idea won't work as we need to separately register tranche for
each process.  The other wayout could be to do it in CreateSharedProcArray()
which will be quite similar to what we do for other tranches and
it will cover all kind of processes.  Attached patch fixes this problem.

I have considered to separately do it in InitProcessPhase2() and
InitAuxiliaryProcess(), but then the registration will be done twice for some
of the processes like bootstrap and same is true if we do this InitProcess()
instead of InitProcessPhase2() and I think it won't be similar to what
we do for other tranches.

I have done the performance testing of the attached patch and the
results are attached with this mail.  The main tests conducted are
pgbench read-write and read-only tests and the results indicate that
this patch doesn't introduce any regression, though you will see some
cases where the performance is better with patch by ~5% and then
regressed by 2~3%, but I think it is more of a noise, then anything
else.


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

pgsql-hackers by date:

Previous
From: Vitaly Burovoy
Date:
Subject: Re: [PROPOSAL] New feature "... VALIDATE CONSTRAINT ... USING INDEX ..."
Next
From: Piotr Stefaniak
Date:
Subject: Re: [PATCH] Add STRICT to some regression test C functions.