Re: [PATCH] Refactoring of LWLock tranches

From: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
To: Ildus Kurbangaliev <i(dot)kurbangaliev(at)postgrespro(dot)ru>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, "andres(at)anarazel(dot)de" <andres(at)anarazel(dot)de>, Simon Riggs <simon(at)2ndquadrant(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>, Alexander Korotkov <a(dot)korotkov(at)postgrespro(dot)ru>, Heikki Linnakangas <hlinnaka(at)iki(dot)fi>
Subject: Re: [PATCH] Refactoring of LWLock tranches
Date: 2016-01-09 04:22:49
Message-ID: CAA4eK1LQKS4hr1i5yOXL9F_D37eSdrUg_1hsLjSG7uHSptutDw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox
Thread:
Lists: pgsql-hackers

On Mon, Dec 28, 2015 at 4:47 PM, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
wrote:

> On Thu, Dec 24, 2015 at 5:50 PM, Ildus Kurbangaliev <
> i(dot)kurbangaliev(at)postgrespro(dot)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 Content-Type Size
proc_lwlocks_tranche_v2.patch application/octet-stream 11.2 KB
perf_proc_lwlock_tranche_reads.png image/png 80.2 KB
image/png 64.1 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Piotr Stefaniak 2016-01-09 07:08:22 Re: [PATCH] Add STRICT to some regression test C functions.
Previous Message Vitaly Burovoy 2016-01-09 03:08:52 Re: [PROPOSAL] New feature "... VALIDATE CONSTRAINT ... USING INDEX ..."