diff mbox series

[v4,bpf-next,2/2] selftests/bpf: refactor bench reporting functions

Message ID 20220530202711.2594486-2-davemarchevsky@fb.com (mailing list archive)
State Superseded
Delegated to: BPF
Headers show
Series [v4,bpf-next,1/2] selftests/bpf: Add benchmark for local_storage get | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for bpf-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix success Link
netdev/cover_letter success Single patches do not need cover letters
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers warning 7 maintainers not CCed: netdev@vger.kernel.org songliubraving@fb.com linux-kselftest@vger.kernel.org yhs@fb.com john.fastabend@gmail.com shuah@kernel.org kpsingh@kernel.org
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch warning WARNING: Macros with flow control statements should be avoided WARNING: line length of 86 exceeds 80 columns WARNING: line length of 87 exceeds 80 columns WARNING: line length of 88 exceeds 80 columns WARNING: line length of 90 exceeds 80 columns WARNING: line length of 98 exceeds 80 columns
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
bpf/vmtest-bpf-next-PR success PR summary
bpf/vmtest-bpf-next-VM_Test-1 success Logs for Kernel LATEST on ubuntu-latest with gcc
bpf/vmtest-bpf-next-VM_Test-2 success Logs for Kernel LATEST on ubuntu-latest with llvm-15
bpf/vmtest-bpf-next-VM_Test-3 success Logs for Kernel LATEST on z15 with gcc

Commit Message

Dave Marchevsky May 30, 2022, 8:27 p.m. UTC 
The report_progress and report_final functions in bench.c do similar
calculation of mean and stddev, but each of these rewrites the entire
calculation. Add helpers which calculate mean and stddev for simple
fields of struct bench_res, or in the case of total_ops, sum of two
fields, and move all extant calculations of mean and stddev to use
the helpers.

Also add some macros for unit conversion constants to improve
readability.

Signed-off-by: Dave Marchevsky <davemarchevsky@fb.com>
---
 tools/testing/selftests/bpf/bench.c | 156 +++++++++++++++++-----------
 1 file changed, 95 insertions(+), 61 deletions(-)

Comments

Andrii Nakryiko June 3, 2022, 9:26 p.m. UTC | #1 
On Mon, May 30, 2022 at 1:27 PM Dave Marchevsky <davemarchevsky@fb.com> wrote:
>
> The report_progress and report_final functions in bench.c do similar
> calculation of mean and stddev, but each of these rewrites the entire
> calculation. Add helpers which calculate mean and stddev for simple
> fields of struct bench_res, or in the case of total_ops, sum of two
> fields, and move all extant calculations of mean and stddev to use
> the helpers.
>
> Also add some macros for unit conversion constants to improve
> readability.
>
> Signed-off-by: Dave Marchevsky <davemarchevsky@fb.com>
> ---
>  tools/testing/selftests/bpf/bench.c | 156 +++++++++++++++++-----------
>  1 file changed, 95 insertions(+), 61 deletions(-)
>

please drop this patch, it doesn't look like an improvement

> diff --git a/tools/testing/selftests/bpf/bench.c b/tools/testing/selftests/bpf/bench.c
> index 32399554f89b..0383bcd0e009 100644
> --- a/tools/testing/selftests/bpf/bench.c
> +++ b/tools/testing/selftests/bpf/bench.c
> @@ -28,6 +28,62 @@ static int libbpf_print_fn(enum libbpf_print_level level,
>         return vfprintf(stderr, format, args);
>  }
>

[...]
diff mbox series

Patch

diff --git a/tools/testing/selftests/bpf/bench.c b/tools/testing/selftests/bpf/bench.c
index 32399554f89b..0383bcd0e009 100644
--- a/tools/testing/selftests/bpf/bench.c
+++ b/tools/testing/selftests/bpf/bench.c
@@ -28,6 +28,62 @@  static int libbpf_print_fn(enum libbpf_print_level level,
 	return vfprintf(stderr, format, args);
 }
 
+#define benchres_getter(field) bench_res__ ## field
+#define DEFINE_BENCHRES_GETTER(field) \
+static long benchres_getter(field)(struct bench_res *res) { return res->field; }
+
+DEFINE_BENCHRES_GETTER(hits);
+DEFINE_BENCHRES_GETTER(drops);
+DEFINE_BENCHRES_GETTER(important_hits);
+static long benchres_getter(total_ops)(struct bench_res *res)
+{
+	return res->hits + res->drops;
+}
+
+static double calc_mean_long(struct bench_res res[], int res_cnt,
+			     long (*fn)(struct bench_res *res))
+{
+	double sum;
+	int i;
+
+	for (i = 0, sum = 0; i < res_cnt; i++)
+		sum += fn(&res[i]);
+	return sum / res_cnt;
+}
+
+#define calc_mean_hits(res, res_cnt) calc_mean_long(res, res_cnt, benchres_getter(hits))
+#define calc_mean_drops(res, res_cnt) calc_mean_long(res, res_cnt, benchres_getter(drops))
+#define calc_mean_important_hits(res, res_cnt) \
+	calc_mean_long(res, res_cnt, benchres_getter(important_hits))
+#define calc_mean_total_ops(res, res_cnt) calc_mean_long(res, res_cnt, benchres_getter(total_ops))
+
+static double calc_stddev_long(struct bench_res res[], int res_cnt,
+			       double mean, long (*fn)(struct bench_res *res))
+{
+	double sum;
+	long val;
+	int i;
+
+	for (i = 0, sum = 0; i < res_cnt; i++) {
+		val = fn(&res[i]);
+		sum += (mean - val) * (mean - val) / (res_cnt - 1.0);
+	}
+	return sqrt(sum);
+}
+
+#define calc_stddev_hits(res, res_cnt, mean) \
+	calc_stddev_long(res, res_cnt, mean, benchres_getter(hits))
+#define calc_stddev_drops(res, res_cnt, mean) \
+	calc_stddev_long(res, res_cnt, mean, benchres_getter(drops))
+#define calc_stddev_important_hits(res, res_cnt, mean) \
+	calc_stddev_long(res, res_cnt, mean, benchres_getter(important_hits))
+#define calc_stddev_total_ops(res, res_cnt, mean) \
+	calc_stddev_long(res, res_cnt, mean, benchres_getter(total_ops))
+
+#define NS_IN_SEC   1000000000
+#define NS_IN_SEC_F 1000000000.0
+#define MILLION_F   1000000.0
+
 void setup_libbpf(void)
 {
 	libbpf_set_strict_mode(LIBBPF_STRICT_ALL);
@@ -39,7 +95,7 @@  void false_hits_report_progress(int iter, struct bench_res *res, long delta_ns)
 	long total = res->false_hits  + res->hits + res->drops;
 
 	printf("Iter %3d (%7.3lfus): ",
-	       iter, (delta_ns - 1000000000) / 1000.0);
+	       iter, (delta_ns - NS_IN_SEC) / 1000.0);
 
 	printf("%ld false hits of %ld total operations. Percentage = %2.2f %%\n",
 	       res->false_hits, total, ((float)res->false_hits / total) * 100);
@@ -68,12 +124,12 @@  void hits_drops_report_progress(int iter, struct bench_res *res, long delta_ns)
 	double hits_per_sec, drops_per_sec;
 	double hits_per_prod;
 
-	hits_per_sec = res->hits / 1000000.0 / (delta_ns / 1000000000.0);
+	hits_per_sec = res->hits / MILLION_F / (delta_ns / NS_IN_SEC_F);
 	hits_per_prod = hits_per_sec / env.producer_cnt;
-	drops_per_sec = res->drops / 1000000.0 / (delta_ns / 1000000000.0);
+	drops_per_sec = res->drops / MILLION_F / (delta_ns / NS_IN_SEC_F);
 
 	printf("Iter %3d (%7.3lfus): ",
-	       iter, (delta_ns - 1000000000) / 1000.0);
+	       iter, (delta_ns - NS_IN_SEC) / 1000.0);
 
 	printf("hits %8.3lfM/s (%7.3lfM/prod), drops %8.3lfM/s, total operations %8.3lfM/s\n",
 	       hits_per_sec, hits_per_prod, drops_per_sec, hits_per_sec + drops_per_sec);
@@ -81,34 +137,26 @@  void hits_drops_report_progress(int iter, struct bench_res *res, long delta_ns)
 
 void hits_drops_report_final(struct bench_res res[], int res_cnt)
 {
-	int i;
 	double hits_mean = 0.0, drops_mean = 0.0, total_ops_mean = 0.0;
 	double hits_stddev = 0.0, drops_stddev = 0.0, total_ops_stddev = 0.0;
-	double total_ops;
 
-	for (i = 0; i < res_cnt; i++) {
-		hits_mean += res[i].hits / 1000000.0 / (0.0 + res_cnt);
-		drops_mean += res[i].drops / 1000000.0 / (0.0 + res_cnt);
-	}
-	total_ops_mean = hits_mean + drops_mean;
+	hits_mean = calc_mean_hits(res, res_cnt);
+	drops_mean = calc_mean_drops(res, res_cnt);
+	total_ops_mean = calc_mean_total_ops(res, res_cnt);
 
 	if (res_cnt > 1)  {
-		for (i = 0; i < res_cnt; i++) {
-			hits_stddev += (hits_mean - res[i].hits / 1000000.0) *
-				       (hits_mean - res[i].hits / 1000000.0) /
-				       (res_cnt - 1.0);
-			drops_stddev += (drops_mean - res[i].drops / 1000000.0) *
-					(drops_mean - res[i].drops / 1000000.0) /
-					(res_cnt - 1.0);
-			total_ops = res[i].hits + res[i].drops;
-			total_ops_stddev += (total_ops_mean - total_ops / 1000000.0) *
-					(total_ops_mean - total_ops / 1000000.0) /
-					(res_cnt - 1.0);
-		}
-		hits_stddev = sqrt(hits_stddev);
-		drops_stddev = sqrt(drops_stddev);
-		total_ops_stddev = sqrt(total_ops_stddev);
+		hits_stddev = calc_stddev_hits(res, res_cnt, hits_mean);
+		drops_stddev = calc_stddev_drops(res, res_cnt, drops_mean);
+		total_ops_stddev = calc_stddev_total_ops(res, res_cnt, total_ops_mean);
 	}
+
+	hits_mean /= MILLION_F;
+	drops_mean /= MILLION_F;
+	total_ops_mean /= MILLION_F;
+	hits_stddev /= MILLION_F;
+	drops_stddev /= MILLION_F;
+	total_ops_stddev /= MILLION_F;
+
 	printf("Summary: hits %8.3lf \u00B1 %5.3lfM/s (%7.3lfM/prod), ",
 	       hits_mean, hits_stddev, hits_mean / env.producer_cnt);
 	printf("drops %8.3lf \u00B1 %5.3lfM/s, ",
@@ -121,10 +169,10 @@  void ops_report_progress(int iter, struct bench_res *res, long delta_ns)
 {
 	double hits_per_sec, hits_per_prod;
 
-	hits_per_sec = res->hits / 1000000.0 / (delta_ns / 1000000000.0);
+	hits_per_sec = res->hits / MILLION_F / (delta_ns / NS_IN_SEC_F);
 	hits_per_prod = hits_per_sec / env.producer_cnt;
 
-	printf("Iter %3d (%7.3lfus): ", iter, (delta_ns - 1000000000) / 1000.0);
+	printf("Iter %3d (%7.3lfus): ", iter, (delta_ns - NS_IN_SEC) / 1000.0);
 
 	printf("hits %8.3lfM/s (%7.3lfM/prod)\n", hits_per_sec, hits_per_prod);
 }
@@ -132,19 +180,13 @@  void ops_report_progress(int iter, struct bench_res *res, long delta_ns)
 void ops_report_final(struct bench_res res[], int res_cnt)
 {
 	double hits_mean = 0.0, hits_stddev = 0.0;
-	int i;
-
-	for (i = 0; i < res_cnt; i++)
-		hits_mean += res[i].hits / 1000000.0 / (0.0 + res_cnt);
 
-	if (res_cnt > 1)  {
-		for (i = 0; i < res_cnt; i++)
-			hits_stddev += (hits_mean - res[i].hits / 1000000.0) *
-				       (hits_mean - res[i].hits / 1000000.0) /
-				       (res_cnt - 1.0);
+	hits_mean = calc_mean_hits(res, res_cnt);
+	if (res_cnt > 1)
+		hits_stddev = calc_stddev_hits(res, res_cnt, hits_mean);
+	hits_mean /= MILLION_F;
+	hits_stddev /= MILLION_F;
 
-		hits_stddev = sqrt(hits_stddev);
-	}
 	printf("Summary: throughput %8.3lf \u00B1 %5.3lf M ops/s (%7.3lfM ops/prod), ",
 	       hits_mean, hits_stddev, hits_mean / env.producer_cnt);
 	printf("latency %8.3lf ns/op\n", 1000.0 / hits_mean * env.producer_cnt);
@@ -154,12 +196,12 @@  void local_storage_report_progress(int iter, struct bench_res *res,
 				   long delta_ns)
 {
 	double important_hits_per_sec, hits_per_sec;
-	double delta_sec = delta_ns / 1000000000.0;
+	double delta_sec = delta_ns / NS_IN_SEC_F;
 
-	hits_per_sec = res->hits / 1000000.0 / delta_sec;
-	important_hits_per_sec = res->important_hits / 1000000.0 / delta_sec;
+	hits_per_sec = res->hits / MILLION_F / delta_sec;
+	important_hits_per_sec = res->important_hits / MILLION_F / delta_sec;
 
-	printf("Iter %3d (%7.3lfus): ", iter, (delta_ns - 1000000000) / 1000.0);
+	printf("Iter %3d (%7.3lfus): ", iter, (delta_ns - NS_IN_SEC) / 1000.0);
 
 	printf("hits %8.3lfM/s ", hits_per_sec);
 	printf("important_hits %8.3lfM/s\n", important_hits_per_sec);
@@ -169,27 +211,19 @@  void local_storage_report_final(struct bench_res res[], int res_cnt)
 {
 	double important_hits_mean = 0.0, important_hits_stddev = 0.0;
 	double hits_mean = 0.0, hits_stddev = 0.0;
-	int i;
 
-	for (i = 0; i < res_cnt; i++) {
-		hits_mean += res[i].hits / 1000000.0 / (0.0 + res_cnt);
-		important_hits_mean += res[i].important_hits / 1000000.0 / (0.0 + res_cnt);
+	hits_mean = calc_mean_hits(res, res_cnt);
+	important_hits_mean = calc_mean_important_hits(res, res_cnt);
+	if (res_cnt > 1) {
+		hits_stddev = calc_stddev_hits(res, res_cnt, hits_mean);
+		important_hits_stddev =
+			calc_stddev_important_hits(res, res_cnt, important_hits_mean);
 	}
+	hits_mean /= MILLION_F;
+	important_hits_mean /= MILLION_F;
+	hits_stddev /= MILLION_F;
+	important_hits_stddev /= MILLION_F;
 
-	if (res_cnt > 1)  {
-		for (i = 0; i < res_cnt; i++) {
-			hits_stddev += (hits_mean - res[i].hits / 1000000.0) *
-				       (hits_mean - res[i].hits / 1000000.0) /
-				       (res_cnt - 1.0);
-			important_hits_stddev +=
-				       (important_hits_mean - res[i].important_hits / 1000000.0) *
-				       (important_hits_mean - res[i].important_hits / 1000000.0) /
-				       (res_cnt - 1.0);
-		}
-
-		hits_stddev = sqrt(hits_stddev);
-		important_hits_stddev = sqrt(important_hits_stddev);
-	}
 	printf("Summary: hits throughput %8.3lf \u00B1 %5.3lf M ops/s, ",
 	       hits_mean, hits_stddev);
 	printf("hits latency %8.3lf ns/op, ", 1000.0 / hits_mean);