Subject: Re: [PATCH v4 4/5] meson: wire up benchmarks

From: Patrick Steinhardt <[email-masked]>
Date: Fri, 25 Apr 2025 10:27:03 +0200

On Fri, Apr 25, 2025 at 10:09:18AM +0200, Christian Couder wrote:
> On Fri, Apr 25, 2025 at 10:01 AM Patrick Steinhardt <[email-masked]> wrote:
> >
> > On Fri, Apr 25, 2025 at 09:57:33AM +0200, Christian Couder wrote:
> > > On Fri, Apr 25, 2025 at 9:28 AM Patrick Steinhardt <[email-masked]> wrote:
> > > >
> > > > Wire up benchmarks in Meson. The setup is mostly the same as how we wire
> > > > up our tests. The only difference is that benchmarks get wired up via
> > > > the `benchmark()` option instead of via `test()`, which gives them a bit
> > > > of special treatment:
> > > >
> > > >   - Benchmarks never run in parallel.
> > > >
> > > >   - Benchmarks aren't run by default when tests are executed.
> > > >
> > > >   - Meson does not inject the `MALLOC_PERTURB` environment variable.
> > >
> > > [...]
> > >
> > > > diff --git a/meson.build b/meson.build
> > > > index 410bbf93dad..2cd05a9b1ba 100644
> > > > --- a/meson.build
> > > > +++ b/meson.build
> > > > @@ -70,6 +70,12 @@
> > > >  #      # Execute single test interactively such that features like `debug ()` work.
> > > >  #      $ meson test -i --test-args='-ix' t1400-update-ref
> > > >  #
> > > > +#      # Execute all benchmarks.
> > > > +#      $ meson test -i --benchmark
> > > > +#
> > > > +#      # Execute single benchmark.
> > > > +#      $ meson test -i --benchmark p0000-*
> > > > +#
> > > >  #   Test execution is parallelized by default and scales with the number of
> > > >  #   processor cores available. You can change the number of processes by passing
> > > >  #   the `-jN` flag to `meson test`.
> > >
> > > Doesn't the above give the impression that benchmarks are also
> > > parallelized by default while the commit message says they are not?
> >
> > To me it doesn't because it talks about test execution, not benchmark
> > execution. The comments for each of the invocations also discern between
> > tests and benchmarks. I'm happy to hear about alternative ways to word
> > this though.
> 
> Maybe something like:
> 
> "Test execution (but not benchmark execution) is parallelized by default ..."

Okay. I amended this locally, but will hold off sending another
iteration just for this change. Thanks!

Patrick