Re: Add tracking of backend memory allocated to pg_stat_activity

At Tue, 06 Sep 2022 17:10:49 -0400, Reid Thompson <reid(dot)thompson(at)crunchydata(dot)com> wrote in
> On Thu, 2022-09-01 at 13:43 +0900, Kyotaro Horiguchi wrote:
> >
> > > @@ -916,6 +930,7 @@ AllocSetAlloc(MemoryContext context, Size size)
> > >    return NULL;
> > >  
> > >    context->mem_allocated += blksize;
> > > + pgstat_report_backend_mem_allocated_increase(blksi
> > > ze);
> >
> > I'm not sure this is acceptable. The function adds a branch even when
> > the feature is turned off, which I think may cause a certain extent
> > of
> > performance degradation. A past threads [1], [2] and [3] might be
> > informative.
>
> Stated above is '...even when the feature is turned off...', I want to
> note that this feature/patch (for tracking memory allocated) doesn't
> have an 'on/off'. Tracking would always occur.

In the patch, I see that
pgstat_report_backend_mem_allocated_increase() runs the following
code, which seems like to me to be a branch..

+ if (!beentry || !pgstat_track_activities)
+ {
+ /*
+ * Account for memory before pgstats is initialized. This will be
+ * migrated to pgstats on initialization.
+ */
+ backend_mem_allocated += allocation;
+
+ return;
+ }

> I'm open to guidance on testing for performance degradation. I did
> note some basic pgbench comparison numbers in the thread regarding
> limiting backend memory allocations.

Yeah.. That sounds good..

(I have a patch that is stuck at benchmarking on slight possible
degradation caused by a branch (or indirect call) on a hot path
similary to this one. The test showed fluctuation that is not clearly
distinguishable between noise and degradation by running the target
functions in a busy loop..)

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center