On Thu, Sep 7, 2023 at 4:04 PM Michael Paquier <mich...@paquier.xyz> wrote: > > On Thu, Sep 07, 2023 at 01:22:07PM +0530, Ashutosh Bapat wrote: > > 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. > > So, is the difference measurable? Assuming that your compiler does I have checked my compiler, this patch give them same assembly code as before. > not optimize that, my guess is no because the cycles are going to be > eaten by the other system calls in pgrowlocks. You could increase the > number of proc entries and force a large loop around > BackendXidGetPid() to see a difference of runtime, but that's going > to require a lot more proc entries to see any kind of difference > outside the scope of a usual ~1% (somewhat magic number!) noise with > such micro benchmarks. > > - int pgprocno = arrayP->pgprocnos[index]; > - PGPROC *proc = &allProcs[pgprocno]; > - > if (other_xids[index] == xid) > { > + PGPROC *proc = &allProcs[arrayP->pgprocnos[index]]; > result = proc->pid; > break; > > Saying that, moving the declarations into the inner loop is usually a > good practice, but I would just keep two variables instead of one for > the sake of readability. That's a nit, sure.
I remember I split this into two lines in v2 patch. Whatever, I attached a v3 with a suggestion from Ashutosh Bapat. > -- > Michael -- Regards Junwang Zhao
v3-0001-BackendXidGetPid-only-access-allProcs-when-xid-ma.patch
Description: Binary data