From: | Dilip Kumar <dilipbalaut(at)gmail(dot)com> |
---|---|
To: | Kuntal Ghosh <kuntalghosh(dot)2007(at)gmail(dot)com> |
Cc: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Masahiko Sawada <masahiko(dot)sawada(at)2ndquadrant(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Robert Haas <robertmhaas(at)gmail(dot)com>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, Michael Paquier <michael(at)paquier(dot)xyz>, Mithun Cy <mithun(dot)cy(at)enterprisedb(dot)com>, Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: [HACKERS] Moving relation extension locks out of heavyweight lock manager |
Date: | 2020-03-13 10:11:56 |
Message-ID: | CAFiTN-t-NttZBabhKkbVHX7iSdXzmXZHXCVNKddk5Pxfi2WScw@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Fri, Mar 13, 2020 at 2:32 PM Kuntal Ghosh <kuntalghosh(dot)2007(at)gmail(dot)com> wrote:
>
> On Fri, Mar 13, 2020 at 8:29 AM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> >
> > On Thu, Mar 12, 2020 at 7:50 PM Kuntal Ghosh <kuntalghosh(dot)2007(at)gmail(dot)com> wrote:
> > > I think moving them inside a macro is a good idea. Also, I think we
> > > should move all the Assert related code inside some debugging macro
> > > similar to this:
> > > #ifdef LOCK_DEBUG
> > > ....
> > > #endif
> > >
> > If we move it under some macro, then those Asserts will be only
> > enabled when that macro is defined. I think we want there Asserts to
> > be enabled always in assert enabled build, these will be like any
> > other Asserts in the code. What is the advantage of doing those under
> > macro?
> >
> My concern is related to performance regression. We're using two
> static variables in hot-paths only for checking a few asserts. So, I'm
> not sure whether we should enable the same by default, specially when
> asserts are itself disabled.
> -ResetRelExtLockHeldCount()
> +ResetRelExtPageLockHeldCount()
> {
> RelationExtensionLockHeldCount = 0;
> + PageLockHeldCount = 0;
> +}
> Also, we're calling this method from frequently used functions like
> Commit/AbortTransaction. So, it's better these two static variables
> share the same cache line and reinitalize them with a single
> instruction.
In the recent version of the patch, instead of a counter, we have done
with a flag. So I think now we can just keep a single variable and we
can just reset the bit in a single instruction.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
From | Date | Subject | |
---|---|---|---|
Next Message | Isaac Morland | 2020-03-13 11:48:33 | Re: truncating timestamps on arbitrary intervals |
Previous Message | Amit Kapila | 2020-03-13 10:09:42 | Re: [HACKERS] Moving relation extension locks out of heavyweight lock manager |