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