Re: [BackendXidGetPid] only access allProcs when xid matches

Hi Junwang,
We leave a line blank after variable declaration as in the attached patch.

Otherwise the patch looks good to me.

The function modified by the patch is only used by extension
pgrowlocks. Given that the function will be invoked as many times as
the number of locked rows in the relation, the patch may show some
improvement and thus be more compelling. One way to measure
performance is to create a table with millions of rows, SELECT all
rows with FOR SHARE/UPDATE clause. Then run pgrowlock() on that
relation. This will invoke the given function a million times. That
way we might be able to catch some miniscule improvement per row.

If the performance is measurable, we can mark the CF entry as ready
for committer.

--
Best Wishes,
Ashutosh Bapat

On Thu, Aug 10, 2023 at 1:48 PM Junwang Zhao <zhjwpku(at)gmail(dot)com> wrote:
>
> On Thu, Aug 10, 2023 at 4:11 PM Ashutosh Bapat
> <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com> wrote:
> >
> > Please add this to commitfest so that it's not forgotten.
> >
>
> Added [1], thanks
>
> [1]: https://commitfest.postgresql.org/44/4495/
>
> > On Wed, Aug 9, 2023 at 8:37 PM Junwang Zhao <zhjwpku(at)gmail(dot)com> wrote:
> > >
> > > On Wed, Aug 9, 2023 at 10:46 PM Ashutosh Bapat
> > > <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com> wrote:
> > > >
> > > > On Wed, Aug 9, 2023 at 9:30 AM Junwang Zhao <zhjwpku(at)gmail(dot)com> wrote:
> > > > >
> > > > > In function `BackendXidGetPid`, when looping every proc's
> > > > > TransactionId, there is no need to access its PGPROC since there
> > > > > is shared memory access: `arrayP->pgprocnos[index]`.
> > > > >
> > > > > Though the compiler can optimize this kind of inefficiency, I
> > > > > believe we should ship with better code.
> > > > >
> > > >
> > > > Looks good to me. However, I would just move the variable declaration
> > > > with their assignments inside the if () rather than combing the
> > > > expressions. It more readable that way.
> > >
> > > yeah, make sense, also checked elsewhere using the original style,
> > > attachment file
> > > keep that style, thanks ;)
> > >
> > > >
> > > > --
> > > > Best Wishes,
> > > > Ashutosh Bapat
> > >
> > >
> > >
> > > --
> > > Regards
> > > Junwang Zhao
> >
> >
> >
> > --
> > Best Wishes,
> > Ashutosh Bapat
>
>
>
> --
> Regards
> Junwang Zhao