Re: [PATCH v1] perf test: Add a runs-per-test flag

[Liang, Kan]

On 2024-11-09 11:02 a.m., Ian Rogers wrote:
> To detect flakes it is useful to run tests more than once. Add a
> runs-per-test flag that will run each test multiple times.
> 
> Signed-off-by: Ian Rogers <irogers@xxxxxxxxxx>
> ---
>  tools/perf/tests/builtin-test.c | 38 ++++++++++++++++++++-------------
>  1 file changed, 23 insertions(+), 15 deletions(-)
> 
> diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
> index d2cabaa8ad92..574fbd5caff0 100644
> --- a/tools/perf/tests/builtin-test.c
> +++ b/tools/perf/tests/builtin-test.c
> @@ -42,6 +42,8 @@
>  static bool dont_fork;
>  /* Fork the tests in parallel and wait for their completion. */
>  static bool sequential;
> +/* Numer of times each test is run. */
> +static unsigned int runs_per_test = 1;
>  const char *dso_to_test;
>  const char *test_objdump_path = "objdump";
>  
> @@ -490,10 +492,10 @@ static int __cmd_test(struct test_suite **suites, int argc, const char *argv[],
>  				len = strlen(test_description(*t, subi));
>  				if (width < len)
>  					width = len;
> -				num_tests++;
> +				num_tests += runs_per_test;
>  			}
>  		} else {
> -			num_tests++;
> +			num_tests += runs_per_test;
>  		}
>  	}

Seems we just need to calculate the num_tests once at the end for each
loop. Something as below may works. (not tested)

@@ -482,20 +490,19 @@ static int __cmd_test(struct test_suite **suites,
int argc, const char *argv[],

        for (struct test_suite **t = suites; *t; t++) {
                int len = strlen(test_description(*t, -1));
+               int subi = 0, subn = 1;

                if (width < len)
                        width = len;

                if (has_subtests(*t)) {
-                       for (int subi = 0, subn = num_subtests(*t); subi
< subn; subi++) {
+                       for (subn = num_subtests(*t); subi < subn; subi++) {
                                len = strlen(test_description(*t, subi));
                                if (width < len)
                                        width = len;
-                               num_tests++;
                        }
-               } else {
-                       num_tests++;
                }
+               num_tests += subn * runs_per_test;
        }
        child_tests = calloc(num_tests, sizeof(*child_tests));
        if (!child_tests)




>  	child_tests = calloc(num_tests, sizeof(*child_tests));
> @@ -556,21 +558,25 @@ static int __cmd_test(struct test_suite **suites, int argc, const char *argv[],
>  			}
>  
>  			if (!has_subtests(*t)) {
> -				err = start_test(*t, curr, -1, &child_tests[child_test_num++],
> -						 width, pass);
> -				if (err)
> -					goto err_out;
> +				for (unsigned int run = 0; run < runs_per_test; run++) {
> +					err = start_test(*t, curr, -1, &child_tests[child_test_num++],
> +							width, pass);
> +					if (err)
> +						goto err_out;
> +				}
>  				continue;
>  			}
> -			for (int subi = 0, subn = num_subtests(*t); subi < subn; subi++) {
> -				if (!perf_test__matches(test_description(*t, subi),
> -							curr, argc, argv))
> -					continue;
> -
> -				err = start_test(*t, curr, subi, &child_tests[child_test_num++],
> -						 width, pass);
> -				if (err)
> -					goto err_out;
> +			for (unsigned int run = 0; run < runs_per_test; run++) {
> +				for (int subi = 0, subn = num_subtests(*t); subi < subn; subi++) {
> +					if (!perf_test__matches(test_description(*t, subi),
> +									curr, argc, argv))
> +						continue;
> +
> +					err = start_test(*t, curr, subi, &child_tests[child_test_num++],
> +							width, pass);
> +					if (err)
> +						goto err_out;
> +				}

Can we add a wrapper for the start_test()? Something similar to below?
It avoids adding the loop for every places using the start_test.

+static int start_test(struct test_suite *test, int i, int subi, struct
child_test **child,
+               int width, int pass)
+{
+       for (unsigned int run = 0; run < runs_per_test; run++) {
+               __start_test();
+       }
+}

Thanks,
Kan

>  			}
>  		}
>  		if (!sequential) {
> @@ -714,6 +720,8 @@ int cmd_test(int argc, const char **argv)
>  		    "Do not fork for testcase"),
>  	OPT_BOOLEAN('S', "sequential", &sequential,
>  		    "Run the tests one after another rather than in parallel"),
> +	OPT_UINTEGER('r', "runs-per-test", &runs_per_test,
> +		     "Run each test the given number of times, default 1"),
>  	OPT_STRING('w', "workload", &workload, "work", "workload to run for testing, use '--list-workloads' to list the available ones."),
>  	OPT_BOOLEAN(0, "list-workloads", &list_workloads, "List the available builtin workloads to use with -w/--workload"),
>  	OPT_STRING(0, "dso", &dso_to_test, "dso", "dso to test"),