public inbox for [email protected]  

help / color / mirror / Atom feed

From: David Rowley <[email protected]>
To: Daniil Davydov <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: Postgres hackers <[email protected]>
Subject: Re: Get rid of redundant StringInfo accumulation
Date: Tue, 31 Mar 2026 12:53:43 +1300
Message-ID: <CAApHDvqA26O3mnRg=WgASdPrur7bybvGvLesHvnZ0cKQwoEuxA@mail.gmail.com> (raw)
In-Reply-To: <CAJDiXgjPGu_cW9nsvciqdgAkqY5o5ugaxDvo3S-rOcqBPcHCPw@mail.gmail.com>
References: <CAJDiXgi=Vcp1nh2UoZ5=7BCoR+uXoYzqMsfOfga-XxSUHjFj5Q@mail.gmail.com>
	<eok6kgntbec2vo35xp4eky5ef52ttb7trislyev63ty3ltu533@je4bpifl6spb>
	<CAJDiXgjPGu_cW9nsvciqdgAkqY5o5ugaxDvo3S-rOcqBPcHCPw@mail.gmail.com>

On Tue, 31 Mar 2026 at 03:24, Daniil Davydov <[email protected]> wrote:
> Am I missing something?

I have an opinion here too. It's similar to Andres', and I expect you
won't like it:

Looking at the other users of message_level_is_interesting(), every
one of them that's passing a const is using one of the DEBUG levels,
in which case the vast majority of the time we will take the
short-circuit path. All of the direct cases you're adding pass LOG.
Indirectly, I see some DEBUG3s being passed via
KnownAssignedXidsDisplay(), so maybe that's worth it. I don't know.
You've offered us nothing in terms of performance results from patched
vs unpatched. For me, when I see that, I just assume the person wrote
the patch because they can, not because they thought it was a good
idea. Since this place is rife with people using things like static
analysers and AI tools to give them patch ideas, and since those tools
have about zero context as to if the patch is a good idea or not, it's
very much not inspiring to commit any patch that comes with no proof
that it does what it's meant to do, especially one that has a
trade-off that is disadvantageous to people running the standard
logging levels.

If you still think this is a good idea, then you're going to need to
show benchmark results and scripts to prove that the extra overhead of
message_level_is_interesting() is low enough and the advantages of
skipping the extra StringInfo code is good enough to warrant doing
this. If you don't think that's worth your time, then why do you think
it's worth someone else's? It's your patch.

David

view thread (9+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Get rid of redundant StringInfo accumulation
  In-Reply-To: <CAApHDvqA26O3mnRg=WgASdPrur7bybvGvLesHvnZ0cKQwoEuxA@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox