Received-SPF: pass (google.com: domain of linux-kernel+bounces-109134-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Precedence: bulk
MIME-Version: 1.0
References: <20240301174711.2646944-1-irogers@google.com>
In-Reply-To: <20240301174711.2646944-1-irogers@google.com>
From: Ian Rogers <irogers@google.com>
Date: Wed, 20 Mar 2024 08:58:37 -0700
Message-ID: <CAP-5=fU7PNEDgOg=cFKkSMgHe7_Xbi5Y9k6yPoAwOpN-Uwdp7A@mail.gmail.com>
Subject: Re: [PATCH v1] perf tests: Run tests in parallel by default
To: Peter Zijlstra <peterz@infradead.org>, Ingo Molnar <mingo@redhat.com>, 
	Arnaldo Carvalho de Melo <acme@kernel.org>, Namhyung Kim <namhyung@kernel.org>, 
	Mark Rutland <mark.rutland@arm.com>, 
	Alexander Shishkin <alexander.shishkin@linux.intel.com>, Jiri Olsa <jolsa@kernel.org>, 
	Ian Rogers <irogers@google.com>, Adrian Hunter <adrian.hunter@intel.com>, 
	James Clark <james.clark@arm.com>, Kan Liang <kan.liang@linux.intel.com>, 
	linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Fri, Mar 1, 2024 at 9:47=E2=80=AFAM Ian Rogers <irogers@google.com> wrot=
e:
>
> 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@google.com>

Ping.

Thanks,
Ian

> ---
> This change is on top of the test fixes in:
> https://lore.kernel.org/lkml/20240301074639.2260708-1-irogers@google.com/
> ---
>  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-t=
est.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 =3D "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 =3D run_test_child;
>         err =3D 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[], s=
truct intlist *skiplist)
>                         int err =3D 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 alrea=
dy forked children. */
>                                 free(child_tests);
>                                 return err;
>                         }
> @@ -460,7 +460,7 @@ static int __cmd_test(int argc, const char *argv[], s=
truct intlist *skiplist)
>                 }
>         }
>         for (i =3D 0; i < child_test_num; i++) {
> -               if (parallel) {
> +               if (!sequential) {
>                         int ret  =3D 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 paral=
lel"),
>         OPT_STRING('w', "workload", &workload, "work", "workload to run f=
or 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 =3D true;
> +
>         symbol_conf.priv_size =3D sizeof(int);
>         symbol_conf.try_vmlinux_path =3D true;
>
> --
> 2.44.0.278.ge034bb2e1d-goog
>