Date: Sat, 26 Feb 2022 00:01:41 +0800
From: Julien Rouhaud <rjuju123@gmail.com>
To: Magnus Hagander <magnus@hagander.net>
Cc: PostgreSQL Developers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Add parameter jit_warn_above_fraction
Message-ID: <20220225160141.k35qjyysclouw5xc@jrouhaud>
References: <CABUevExR_9ZmkYj-aBvDreDKUinWLBBpORcmTbuPdNb5vGOLtA@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CABUevExR_9ZmkYj-aBvDreDKUinWLBBpORcmTbuPdNb5vGOLtA@mail.gmail.com>
Archived-At: <https://www.postgresql.org/message-id/20220225160141.k35qjyysclouw5xc%40jrouhaud>
Precedence: bulk

Hi,

On Fri, Feb 25, 2022 at 04:16:01PM +0100, Magnus Hagander wrote:
> This patch adds a configuration parameter jit_warn_above_fraction that
> will cause a warning to be logged if the fraction of time spent on
> doing JIT is bigger than the specified one. For example, this can be
> used to track down those cases where JIT ends up taking 90% of the
> query runtime because of bad estimates...

I think that's tremendously useful, huge +1.

Just a few minor nit:

+         A value of 0 (the default)disables the warning.

missing space

+			ereport(WARNING,
+					(errmsg("JIT time was %ld ms of %d ms",
+							jit_time, msecs)));

"JIT time" may a bit obscure for users, how about "JIT total processing time"?"

+			gettext_noop("Sets the fraction of query time spent on JIT before writing"
+						 "a warning to the log."),
+			gettext_noop("Write a message tot he server log if more than this"
+						 "fraction of the query runtime is spent on JIT."
+						 "Zero turns off the warning.")

missing spaces in the concatenated strings.

The rest of the patch looks good to me.