Re: [PATCH v1] perf tests: Run tests in parallel by default

[Ian Rogers]

On Fri, Mar 1, 2024 at 9:47 AM Ian Rogers <irogers@xxxxxxxxxx> wrote:
>
> Switch from running tests sequentially to running in parallel by
> default. Change the opt-in '-p' or '--parallel' flag to '-S' or
> '--sequential'.
>
> On an 8 core tigerlake an address sanitizer run time changes from:
> 326.54user 622.73system 6:59.91elapsed 226%CPU
> to:
> 973.02user 583.98system 3:01.17elapsed 859%CPU
>
> So over twice as fast, saving 4 minutes.
>
> Signed-off-by: Ian Rogers <irogers@xxxxxxxxxx>

Ping.

Thanks,
Ian

> ---
> This change is on top of the test fixes in:
> https://lore.kernel.org/lkml/20240301074639.2260708-1-irogers@xxxxxxxxxx/
> ---
>  tools/perf/tests/builtin-test.c | 17 ++++++++++-------
>  1 file changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
> index ddb2f4e38ea5..73f53b02f733 100644
> --- a/tools/perf/tests/builtin-test.c
> +++ b/tools/perf/tests/builtin-test.c
> @@ -39,8 +39,8 @@
>   * making them easier to debug.
>   */
>  static bool dont_fork;
> -/* Fork the tests in parallel and then wait for their completion. */
> -static bool parallel;
> +/* Don't fork the tests in parallel and wait for their completion. */
> +static bool sequential;
>  const char *dso_to_test;
>  const char *test_objdump_path = "objdump";
>
> @@ -374,7 +374,7 @@ static int start_test(struct test_suite *test, int i, int subi, struct child_tes
>         }
>         (*child)->process.no_exec_cmd = run_test_child;
>         err = start_command(&(*child)->process);
> -       if (err || parallel)
> +       if (err || !sequential)
>                 return  err;
>         return finish_test(*child, width);
>  }
> @@ -440,7 +440,7 @@ static int __cmd_test(int argc, const char *argv[], struct intlist *skiplist)
>                         int err = start_test(t, curr, -1, &child_tests[child_test_num++], width);
>
>                         if (err) {
> -                               /* TODO: if parallel waitpid the already forked children. */
> +                               /* TODO: if !sequential waitpid the already forked children. */
>                                 free(child_tests);
>                                 return err;
>                         }
> @@ -460,7 +460,7 @@ static int __cmd_test(int argc, const char *argv[], struct intlist *skiplist)
>                 }
>         }
>         for (i = 0; i < child_test_num; i++) {
> -               if (parallel) {
> +               if (!sequential) {
>                         int ret  = finish_test(child_tests[i], width);
>
>                         if (ret)
> @@ -536,8 +536,8 @@ int cmd_test(int argc, const char **argv)
>                     "be more verbose (show symbol address, etc)"),
>         OPT_BOOLEAN('F', "dont-fork", &dont_fork,
>                     "Do not fork for testcase"),
> -       OPT_BOOLEAN('p', "parallel", &parallel,
> -                   "Run the tests altogether in parallel"),
> +       OPT_BOOLEAN('S', "sequential", &sequential,
> +                   "Run the tests one after another rather than in parallel"),
>         OPT_STRING('w', "workload", &workload, "work", "workload to run for testing"),
>         OPT_STRING(0, "dso", &dso_to_test, "dso", "dso to test"),
>         OPT_STRING(0, "objdump", &test_objdump_path, "path",
> @@ -564,6 +564,9 @@ int cmd_test(int argc, const char **argv)
>         if (workload)
>                 return run_workload(workload, argc, argv);
>
> +       if (dont_fork)
> +               sequential = true;
> +
>         symbol_conf.priv_size = sizeof(int);
>         symbol_conf.try_vmlinux_path = true;
>
> --
> 2.44.0.278.ge034bb2e1d-goog
>