Re: Add tracking of backend memory allocated to pg_stat_activity

At Tue, 06 Sep 2022 17:10:49 -0400, Reid Thompson  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