Hi,
On Mon, Mar 30, 2026 at 7:52 PM Andres Freund <andres@anarazel.de> wrote:
>
> I don't see when the overhead of creating an populating the string info ever
> matters in these cases. This is optimizing something that never can matter for
> real world performance. Even if it were worth optimizing them, I doubt that
> the log level check is useful here, because most if not all of these are
> logged with a level that's logged in nearly all installations.
>
Thank you for the review!
I have a few arguments to defend this patch :
1)
It is normal practice in the code base to check log level before allocating
memory for StringInfo or some other structure. For example, I found the log
level check even before a very lightweight piece of code (see xlogutils.c)
and even if we are going to emit log with DEBUG3 level (see postmaster.c).
2)
Originally, I created this patch in order to avoid spending time during
acquiring LWLock before entering the GetLockHoldersAndWaiters function. The
log_lock_waits parameter is "true" by default, so even if log level is high,
we will *waste* time on the lock acquiring. I.e. this patch is not only about
getting rid of redundant memory allocation - it is also about reducing waiting
time on the locks. I think that it may be noticeable in conditions of a huge
number of parallel processes.
3)
I don't think that all other places (except lock.c and proc.c) where I have
added log level check are really matter for real world performance. This is
more about consistent approach : if we check log level in lock.c, then we
should check it everywhere (if it makes sense). Again, it is normal practice
for the postgres' code.
Am I missing something?
BTW, during writing it I noticed that I forgot to add log level check for
pretty important code path (proc.c). Please, see it in the v3 patch.
--
Best regards,
Daniil Davydov
