[v5,1/1] app/test-compress-perf: report header improvement

Message ID 20190717110247.26594-2-arturx.trybula@intel.com (mailing list archive)
State Accepted, archived
Delegated to: akhil goyal
Headers
Series add extra features to test-compress-perf |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation fail apply issues

Commit Message

Artur Trybula July 17, 2019, 11:02 a.m. UTC
  This patch adds extra features to the compress performance
test. Some important parameters (memory allocation,
number of ops, number of segments) are calculated and
printed out on the screen.
Information about compression threads is also provided.

Signed-off-by: Artur Trybula <arturx.trybula@intel.com>
---
 .../comp_perf_test_benchmark.c                | 21 ++++-
 .../comp_perf_test_common.c                   | 94 ++++++++++++++++++-
 .../comp_perf_test_common.h                   |  6 ++
 app/test-compress-perf/main.c                 |  4 +-
 4 files changed, 120 insertions(+), 5 deletions(-)
  

Comments

Shally Verma July 17, 2019, 11:15 a.m. UTC | #1
> -----Original Message-----
> From: Artur Trybula <arturx.trybula@intel.com>
> Sent: Wednesday, July 17, 2019 4:33 PM
> To: dev@dpdk.org; fiona.trahe@intel.com; Shally Verma
> <shallyv@marvell.com>; adamx.dybkowski@intel.com;
> arturx.trybula@intel.com; akhil.goyal@nxp.com
> Subject: [EXT] [PATCH v5 1/1] app/test-compress-perf: report header
> improvement
> 
> External Email
> 
...

> +static struct cperf_buffer_info tests_res;
Rename test_res too --> buffer_info? You can address it in next release cycle as well.

....
  
Artur Trybula July 17, 2019, 11:26 a.m. UTC | #2
-----Original Message-----
From: Shally Verma [mailto:shallyv@marvell.com] 
Sent: Wednesday, July 17, 2019 13:16
To: Trybula, ArturX <arturx.trybula@intel.com>; dev@dpdk.org; Trahe, Fiona <fiona.trahe@intel.com>; Dybkowski, AdamX <adamx.dybkowski@intel.com>; akhil.goyal@nxp.com
Subject: RE: [EXT] [PATCH v5 1/1] app/test-compress-perf: report header improvement



> -----Original Message-----
> From: Artur Trybula <arturx.trybula@intel.com>
> Sent: Wednesday, July 17, 2019 4:33 PM
> To: dev@dpdk.org; fiona.trahe@intel.com; Shally Verma 
> <shallyv@marvell.com>; adamx.dybkowski@intel.com; 
> arturx.trybula@intel.com; akhil.goyal@nxp.com
> Subject: [EXT] [PATCH v5 1/1] app/test-compress-perf: report header 
> improvement
> 
> External Email
> 
...

> +static struct cperf_buffer_info tests_res;
Rename test_res too --> buffer_info? You can address it in next release cycle as well.
[Artur] Thank you Shally. Good point. Will be changed in the next cycle.

....
  
Akhil Goyal July 19, 2019, 1:17 p.m. UTC | #3
> 
> > +static struct cperf_buffer_info tests_res;
> Rename test_res too --> buffer_info? You can address it in next release cycle as
> well.
> [Artur] Thank you Shally. Good point. Will be changed in the next cycle.
> 
> ....
Applied to dpdk-next-crypto

Thanks.
  
Thomas Monjalon July 19, 2019, 4:21 p.m. UTC | #4
19/07/2019 15:17, Akhil Goyal:
> > 
> > > +static struct cperf_buffer_info tests_res;
> > Rename test_res too --> buffer_info? You can address it in next release cycle as
> > well.
> > [Artur] Thank you Shally. Good point. Will be changed in the next cycle.
> > 
> > ....
> Applied to dpdk-next-crypto

Not pulled in master tree for 2 reasons:
- the title and commit log are not descriptive enough
- there are some UTF-8 characters for no good reason in the printf.
Please stick to standard dash or asterisk for lists,
so it will be printed in any console.
  
Fiona Trahe July 19, 2019, 5:37 p.m. UTC | #5
Hi Thomas,

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> Sent: Friday, July 19, 2019 5:21 PM
> To: Trybula, ArturX <arturx.trybula@intel.com>
> Cc: dev@dpdk.org; Akhil Goyal <akhil.goyal@nxp.com>; Shally Verma <shallyv@marvell.com>; Trahe,
> Fiona <fiona.trahe@intel.com>; Dybkowski, AdamX <adamx.dybkowski@intel.com>
> Subject: Re: [dpdk-dev] [EXT] [PATCH v5 1/1] app/test-compress-perf: report header improvement
> 
> 19/07/2019 15:17, Akhil Goyal:
> > >
> > > > +static struct cperf_buffer_info tests_res;
> > > Rename test_res too --> buffer_info? You can address it in next release cycle as
> > > well.
> > > [Artur] Thank you Shally. Good point. Will be changed in the next cycle.
> > >
> > > ....
> > Applied to dpdk-next-crypto
> 
> Not pulled in master tree for 2 reasons:
> - the title and commit log are not descriptive enough
> - there are some UTF-8 characters for no good reason in the printf.
> Please stick to standard dash or asterisk for lists,
> so it will be printed in any console.
[Fiona] Sorry about this - we didn't know of this requirement.
Will send a v6 next week.
  

Patch

diff --git a/app/test-compress-perf/comp_perf_test_benchmark.c b/app/test-compress-perf/comp_perf_test_benchmark.c
index aa1f8eea2..887459449 100644
--- a/app/test-compress-perf/comp_perf_test_benchmark.c
+++ b/app/test-compress-perf/comp_perf_test_benchmark.c
@@ -329,9 +329,26 @@  cperf_benchmark_test_runner(void *test_ctx)
 	struct comp_test_data *test_data = ctx->ver.options;
 	uint32_t lcore = rte_lcore_id();
 	static rte_atomic16_t display_once = RTE_ATOMIC16_INIT(0);
+	int i, ret = EXIT_SUCCESS;
 
 	ctx->ver.mem.lcore_id = lcore;
-	int i, ret = EXIT_SUCCESS;
+
+	/*
+	 * printing information about current compression thread
+	 */
+	if (rte_atomic16_test_and_set(&ctx->ver.mem.print_info_once))
+		printf("    lcore: %u,"
+				" driver name: %s,"
+				" device name: %s,"
+				" device id: %u,"
+				" socket id: %u,"
+				" queue pair id: %u\n",
+			lcore,
+			ctx->ver.options->driver_name,
+			rte_compressdev_name_get(ctx->ver.mem.dev_id),
+			ctx->ver.mem.dev_id,
+			rte_compressdev_socket_id(ctx->ver.mem.dev_id),
+			ctx->ver.mem.qp_id);
 
 	/*
 	 * First the verification part is needed
@@ -374,7 +391,7 @@  cperf_benchmark_test_runner(void *test_ctx)
 			1000000000;
 
 	if (rte_atomic16_test_and_set(&display_once)) {
-		printf("%12s%6s%12s%17s%15s%16s\n",
+		printf("\n%12s%6s%12s%17s%15s%16s\n",
 			"lcore id", "Level", "Comp size", "Comp ratio [%]",
 			"Comp [Gbps]", "Decomp [Gbps]");
 	}
diff --git a/app/test-compress-perf/comp_perf_test_common.c b/app/test-compress-perf/comp_perf_test_common.c
index 472c76686..fab514e50 100644
--- a/app/test-compress-perf/comp_perf_test_common.c
+++ b/app/test-compress-perf/comp_perf_test_common.c
@@ -16,6 +16,18 @@ 
 
 #define DIV_CEIL(a, b)  ((a) / (b) + ((a) % (b) != 0))
 
+struct cperf_buffer_info {
+	uint16_t total_segments;
+	uint16_t segment_sz;
+	uint16_t last_segment_sz;
+	uint32_t total_buffs;	      /*number of buffers = number of ops*/
+	uint16_t segments_per_buff;
+	uint16_t segments_per_last_buff;
+	size_t input_data_sz;
+};
+
+static struct cperf_buffer_info tests_res;
+
 int
 param_range_check(uint16_t size, const struct rte_param_log2_range *range)
 {
@@ -170,6 +182,13 @@  comp_perf_allocate_memory(struct comp_test_data *test_data,
 				" could not be allocated\n");
 		return -1;
 	}
+
+	tests_res.total_segments = total_segs;
+	tests_res.segment_sz = test_data->seg_sz;
+	tests_res.total_buffs = mem->total_bufs;
+	tests_res.segments_per_buff = test_data->max_sgl_segs;
+	tests_res.input_data_sz = test_data->input_data_sz;
+
 	return 0;
 }
 
@@ -178,9 +197,10 @@  prepare_bufs(struct comp_test_data *test_data, struct cperf_mem_resources *mem)
 {
 	uint32_t remaining_data = test_data->input_data_sz;
 	uint8_t *input_data_ptr = test_data->input_data;
-	size_t data_sz;
+	size_t data_sz = 0;
 	uint8_t *data_addr;
 	uint32_t i, j;
+	uint16_t segs_per_mbuf = 0;
 
 	for (i = 0; i < mem->total_bufs; i++) {
 		/* Allocate data in input mbuf and copy data from input file */
@@ -204,7 +224,7 @@  prepare_bufs(struct comp_test_data *test_data, struct cperf_mem_resources *mem)
 		remaining_data -= data_sz;
 
 		/* Already one segment in the mbuf */
-		uint16_t segs_per_mbuf = 1;
+		segs_per_mbuf = 1;
 
 		/* Chain mbufs if needed for input mbufs */
 		while (segs_per_mbuf < test_data->max_sgl_segs
@@ -281,5 +301,75 @@  prepare_bufs(struct comp_test_data *test_data, struct cperf_mem_resources *mem)
 		}
 	}
 
+	tests_res.segments_per_last_buff = segs_per_mbuf;
+	tests_res.last_segment_sz = data_sz;
+
 	return 0;
 }
+
+void
+print_test_dynamics(void)
+{
+	uint32_t opt_total_segs = DIV_CEIL(tests_res.input_data_sz,
+			MAX_SEG_SIZE);
+
+	if (tests_res.total_buffs > 1) {
+		printf("\nWarning: for the current input parameters, number"
+				" of ops is higher than one, which may result"
+				" in sub-optimal performance.\n");
+		printf("To improve the performance (for the current"
+				" input data) following parameters are"
+				" suggested:\n");
+		printf("	• Segment size: %d\n", MAX_SEG_SIZE);
+		printf("	• Number of segments: %u\n", opt_total_segs);
+	} else if (tests_res.total_buffs == 1) {
+		printf("\nInfo: there is only one op with %u segments –"
+				" the compression ratio is the best.\n",
+			tests_res.segments_per_last_buff);
+		if (tests_res.segment_sz < MAX_SEG_SIZE)
+			printf("To reduce compression time, please use"
+					" bigger segment size: %d.\n",
+				MAX_SEG_SIZE);
+		else if (tests_res.segment_sz == MAX_SEG_SIZE)
+			printf("Segment size is optimal for the best"
+					" performance.\n");
+	} else
+		printf("Warning: something wrong happened!!\n");
+
+	printf("\nFor the current input parameters (segment size = %u,"
+			" maximum segments per SGL = %u):\n",
+		tests_res.segment_sz,
+		tests_res.segments_per_buff);
+	printf("	• Total number of buffers: %d\n",
+		tests_res.total_segments);
+	printf("	• %u buffer(s) %u bytes long, last buffer %u"
+			" byte(s) long\n",
+		tests_res.total_segments - 1,
+		tests_res.segment_sz,
+		tests_res.last_segment_sz);
+	printf("	• Number of ops: %u\n", tests_res.total_buffs);
+	printf("	• Total memory allocation: %u\n",
+		(tests_res.total_segments - 1) * tests_res.segment_sz
+		+ tests_res.last_segment_sz);
+	if (tests_res.total_buffs > 1)
+		printf("	• %u ops: %u segment(s) in each,"
+				" segment size %u\n",
+			tests_res.total_buffs - 1,
+			tests_res.segments_per_buff,
+			tests_res.segment_sz);
+	if (tests_res.segments_per_last_buff > 1) {
+		printf("	• 1 op %u segments:\n",
+				tests_res.segments_per_last_buff);
+		printf("		o %u segment size %u\n",
+			tests_res.segments_per_last_buff - 1,
+			tests_res.segment_sz);
+		printf("		o last segment size %u\n",
+			tests_res.last_segment_sz);
+	} else if (tests_res.segments_per_last_buff == 1) {
+		printf("	• 1 op (the last one): %u segment %u"
+				" byte(s) long\n\n",
+			tests_res.segments_per_last_buff,
+			tests_res.last_segment_sz);
+	}
+	printf("\n");
+}
diff --git a/app/test-compress-perf/comp_perf_test_common.h b/app/test-compress-perf/comp_perf_test_common.h
index 9c11e3a00..c9e0c9081 100644
--- a/app/test-compress-perf/comp_perf_test_common.h
+++ b/app/test-compress-perf/comp_perf_test_common.h
@@ -13,6 +13,9 @@  struct cperf_mem_resources {
 	uint8_t dev_id;
 	uint16_t qp_id;
 	uint8_t lcore_id;
+
+	rte_atomic16_t print_info_once;
+
 	uint32_t total_bufs;
 	uint8_t *compressed_data;
 	uint8_t *decompressed_data;
@@ -38,4 +41,7 @@  comp_perf_allocate_memory(struct comp_test_data *test_data,
 int
 prepare_bufs(struct comp_test_data *test_data, struct cperf_mem_resources *mem);
 
+void
+print_test_dynamics(void);
+
 #endif /* _COMP_PERF_TEST_COMMON_H_ */
diff --git a/app/test-compress-perf/main.c b/app/test-compress-perf/main.c
index e746e4708..e7ac412e6 100644
--- a/app/test-compress-perf/main.c
+++ b/app/test-compress-perf/main.c
@@ -363,7 +363,7 @@  main(int argc, char **argv)
 
 	printf("App uses socket: %u\n", rte_socket_id());
 	printf("Burst size = %u\n", test_data->burst_sz);
-	printf("File size = %zu\n", test_data->input_data_sz);
+	printf("Input data size = %zu\n", test_data->input_data_sz);
 
 	test_data->cleanup = ST_DURING_TEST;
 	total_nb_qps = nb_compressdevs * test_data->nb_qps;
@@ -390,6 +390,8 @@  main(int argc, char **argv)
 		i++;
 	}
 
+	print_test_dynamics(); /* constructors must be executed first */
+
 	while (test_data->level <= test_data->level_lst.max) {
 
 		i = 0;