Re: [PATCH v3 0/5] meson: wire up support for benchmarks

From: Christian Couder <christian.couder@gmail.com>
Date: Apr 24, 2025, 06:28 UTC
Message-ID: <CAP8UFD26q4zDTN=NrCtYNFPh+H=f4yU-1+3HxEE-DZD+qO0_Mw@mail.gmail.com>
In-Reply-To: <aAm-pcIfrFt6mjlV@pks.im>

On Thu, Apr 24, 2025 at 6:31 AM Patrick Steinhardt <ps@pks.im> wrote:

Show 5 quoted lines

>
> On Wed, Apr 23, 2025 at 04:44:20PM +0200, Christian Couder wrote:
> > On Tue, Apr 22, 2025 at 9:53 AM Patrick Steinhardt <ps@pks.im> wrote:
> > >
> > > On Tue, Apr 22, 2025 at 09:27:57AM +0200, Christian Couder wrote:

Show 16 quoted lines

> > if time.found()
> >   ...
> > else
> >   error('Benchmarking requires the `time` command')
> > endif
> >
> > in the same way as in meson.build elsewhere we have things like:
> >
> > if not msgfmt.found() and gettext_option.enabled()
> >   error('Internationalization via libintl requires msgfmt')
> > endif
>
> But erroring out by default doesn't really feel nice to the general
> developer. I'd claim that 99% of the time, developers will only end up
> running the test suite, never the benchmarks. So the default should
> match that and not require GNU time to be available, if you ask me.

Yeah, right.

Show 9 quoted lines

> An alternative could be to have a 'benchmarks' feature option wired up.
> This feature would be set to 'auto', and then we can detect time like
> this:
>
>     time = find_program('time', dirs: program_path, required: get_option('benchmarks'))
>
> This will then auto-enable or -disable the feature depending on the
> availability of GNU time. But if the user passes `-Dbenchmarks=enabled`,
> we would fail the setup in case the program wasn't found.

Yeah, that would be great.

An alternative might be a placeholder benchmark that just reports an error when run, like:

if time.found()
   ...
else
  benchmark('perf-benchmarks-require-time', shell, args: [
    '-c',
    'echo "ERROR: Benchmarks require the \"time\" command to be
available. Please install it and reconfigure." >&2; exit 1'
  ], env: test_environment, )
endif