[dpdk-dev] [PATCH] app/test-compress-perf: report header improvement

[dpdk-dev] [PATCH] app/test-compress-perf: report header improvement

[Artur Trybula]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=y, Size: 5878 bytes --]

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.

Signed-off-by: Artur Trybula <arturx.trybula@intel.com>
---
 app/test-compress-perf/main.c | 105 +++++++++++++++++++++++++++++++---
 1 file changed, 98 insertions(+), 7 deletions(-)

diff --git a/app/test-compress-perf/main.c b/app/test-compress-perf/main.c
index c2a45d130..d1578e435 100644
--- a/app/test-compress-perf/main.c
+++ b/app/test-compress-perf/main.c
@@ -27,6 +27,18 @@ static enum cleanup_st {
 	ST_DURING_TEST
 } cleanup = ST_CLEAR;
 
+struct performance_tests_results {
+	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 performance_tests_results tests_res;
+
 static int
 param_range_check(uint16_t size, const struct rte_param_log2_range *range)
 {
@@ -218,6 +230,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 = test_data->total_bufs;
+	tests_res.segments_per_buff = test_data->max_sgl_segs;
+	tests_res.input_data_sz = test_data->input_data_sz;
+
 	return 0;
 }
 
@@ -279,9 +298,11 @@ comp_perf_dump_input_data(struct comp_test_data *test_data)
 
 	if (test_data->input_data_sz > actual_file_sz)
 		RTE_LOG(INFO, USER1,
-		  "%zu bytes read from file %s, extending the file %.2f times\n",
-			test_data->input_data_sz, test_data->input_file,
-			(double)test_data->input_data_sz/actual_file_sz);
+		  "%zu bytes read from file %s, extending the file %.2f times"
+				" to %zu\n",
+			actual_file_sz, test_data->input_file,
+			(double)test_data->input_data_sz/actual_file_sz,
+			test_data->input_data_sz);
 	else
 		RTE_LOG(INFO, USER1,
 			"%zu bytes read from file %s\n",
@@ -349,9 +370,10 @@ prepare_bufs(struct comp_test_data *test_data)
 {
 	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 < test_data->total_bufs; i++) {
 		/* Allocate data in input mbuf and copy data from input file */
@@ -376,7 +398,7 @@ prepare_bufs(struct comp_test_data *test_data)
 		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
@@ -453,6 +475,9 @@ prepare_bufs(struct comp_test_data *test_data)
 		}
 	}
 
+	tests_res.segments_per_last_buff = segs_per_mbuf;
+	tests_res.last_segment_sz = data_sz;
+
 	return 0;
 }
 
@@ -467,7 +492,71 @@ free_bufs(struct comp_test_data *test_data)
 	}
 }
 
-
+static void
+print_report_header(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("\nWarning: 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,"
+			" segments number = %u):\n",
+		tests_res.segment_sz,
+		tests_res.segments_per_buff);
+	printf("	• Total number of segments: %d\n",
+		tests_res.total_segments);
+	printf("	• %u segments %u bytes long, last segment %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 segments 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);
+	}
+}
 
 int
 main(int argc, char **argv)
@@ -533,8 +622,9 @@ main(int argc, char **argv)
 	else
 		level = test_data->level.list[0];
 
+	print_report_header();
+
 	printf("Burst size = %u\n", test_data->burst_sz);
-	printf("File size = %zu\n", test_data->input_data_sz);
 
 	printf("%6s%12s%17s%19s%21s%15s%21s%23s%16s\n",
 		"Level", "Comp size", "Comp ratio [%]",
@@ -612,3 +702,4 @@ main(int argc, char **argv)
 	}
 	return ret;
 }
+
-- 
2.17.1

Re: [dpdk-dev] [PATCH] app/test-compress-perf: report header improvement

[Shally Verma]

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Artur Trybula
> Sent: Monday, June 3, 2019 3:54 PM
> To: dev@dpdk.org; fiona.trahe@intel.com; arturx.trybula@intel.com;
> adamx.dybkowski@intel.com
> Subject: [dpdk-dev] [PATCH] app/test-compress-perf: report header
> improvement
> 
> 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.
> 
> Signed-off-by: Artur Trybula <arturx.trybula@intel.com>
> ---
>  app/test-compress-perf/main.c | 105
> +++++++++++++++++++++++++++++++---
>  1 file changed, 98 insertions(+), 7 deletions(-)
> 
......
> 
> -
> +static void
> +print_report_header(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("\nWarning: 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");

[Shally] Why these kind of warnings? If total_bufs > 1, then, how behavior would change? Same question for segment size as well?


> +	} else
> +		printf("Warning: something wrong happened!!\n");
> +
> +	printf("\nFor the current input parameters (segment size = %u,"
> +			" segments number = %u):\n",
> +		tests_res.segment_sz,
> +		tests_res.segments_per_buff);
> +	printf("	• Total number of segments: %d\n",
> +		tests_res.total_segments);
> +	printf("	• %u segments %u bytes long, last segment %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 segments 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);
> +	}
> +}
> 
>  int
>  main(int argc, char **argv)
> @@ -533,8 +622,9 @@ main(int argc, char **argv)
>  	else
>  		level = test_data->level.list[0];
> 
> +	print_report_header();
> +
[Shally] looks like we're printing input characteristics and possible performance behavior. Is that the intention of this API?

>  	printf("Burst size = %u\n", test_data->burst_sz);
> -	printf("File size = %zu\n", test_data->input_data_sz);
> 
>  	printf("%6s%12s%17s%19s%21s%15s%21s%23s%16s\n",
>  		"Level", "Comp size", "Comp ratio [%]", @@ -612,3 +702,4
> @@ main(int argc, char **argv)
>  	}
>  	return ret;
>  }
> +
> --
> 2.17.1

Re: [dpdk-dev] [PATCH] app/test-compress-perf: report header improvement

[Trybula, ArturX]

-----Original Message-----
From: Shally Verma [mailto:shallyv@marvell.com] 
Sent: Wednesday, June 26, 2019 19:04
To: Trybula, ArturX <arturx.trybula@intel.com>; dev@dpdk.org; Trahe, Fiona <fiona.trahe@intel.com>; Dybkowski, AdamX <adamx.dybkowski@intel.com>
Subject: RE: [dpdk-dev] [PATCH] app/test-compress-perf: report header improvement



> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Artur Trybula
> Sent: Monday, June 3, 2019 3:54 PM
> To: dev@dpdk.org; fiona.trahe@intel.com; arturx.trybula@intel.com; 
> adamx.dybkowski@intel.com
> Subject: [dpdk-dev] [PATCH] app/test-compress-perf: report header 
> improvement
> 
> 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.
> 
> Signed-off-by: Artur Trybula <arturx.trybula@intel.com>
> ---
>  app/test-compress-perf/main.c | 105
> +++++++++++++++++++++++++++++++---
>  1 file changed, 98 insertions(+), 7 deletions(-)
> 
......
> 
> -
> +static void
> +print_report_header(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("\nWarning: 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");

[Shally] Why these kind of warnings? If total_bufs > 1, then, how behavior would change? Same question for segment size as well?
[Artur] I suppose you mean the second one "Warning: There is only one ...". It's just to preserve the structure of the report for both variants.  But I agree, there is nothing wrong if there is only one op.
Please notice that each op is processed independently, in most cases it implies lower compression ratio. The best situation is to have all the input data aggregated into one op (even if partitioned across many mbufs). 

> +	} else
> +		printf("Warning: something wrong happened!!\n");
> +
> +	printf("\nFor the current input parameters (segment size = %u,"
> +			" segments number = %u):\n",
> +		tests_res.segment_sz,
> +		tests_res.segments_per_buff);
> +	printf("	• Total number of segments: %d\n",
> +		tests_res.total_segments);
> +	printf("	• %u segments %u bytes long, last segment %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 segments 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);
> +	}
> +}
> 
>  int
>  main(int argc, char **argv)
> @@ -533,8 +622,9 @@ main(int argc, char **argv)
>  	else
>  		level = test_data->level.list[0];
> 
> +	print_report_header();
> +
[Shally] looks like we're printing input characteristics and possible performance behavior. Is that the intention of this API?
[Artur] That was the idea to have a tool for verification how the data partitioning affects the compression performance. Your description Shally is very accurate: "... printing input characteristics and possible performance behavior" and that is the intention of this API.


>  	printf("Burst size = %u\n", test_data->burst_sz);
> -	printf("File size = %zu\n", test_data->input_data_sz);
> 
>  	printf("%6s%12s%17s%19s%21s%15s%21s%23s%16s\n",
>  		"Level", "Comp size", "Comp ratio [%]", @@ -612,3 +702,4 @@ 
> main(int argc, char **argv)
>  	}
>  	return ret;
>  }
> +
> --
> 2.17.1

Re: [dpdk-dev] [PATCH] app/test-compress-perf: report header improvement

[Shally Verma]

> -----Original Message-----
> From: Trybula, ArturX <arturx.trybula@intel.com>
> Sent: Thursday, June 27, 2019 2:29 PM
> To: Shally Verma <shallyv@marvell.com>; dev@dpdk.org; Trahe, Fiona
> <fiona.trahe@intel.com>; Dybkowski, AdamX
> <adamx.dybkowski@intel.com>
> Subject: [EXT] RE: [dpdk-dev] [PATCH] app/test-compress-perf: report
> header improvement
> 
> External Email
> 
> ----------------------------------------------------------------------
> 
> 
> -----Original Message-----
> From: Shally Verma [mailto:shallyv@marvell.com]
> Sent: Wednesday, June 26, 2019 19:04
> To: Trybula, ArturX <arturx.trybula@intel.com>; dev@dpdk.org; Trahe, Fiona
> <fiona.trahe@intel.com>; Dybkowski, AdamX
> <adamx.dybkowski@intel.com>
> Subject: RE: [dpdk-dev] [PATCH] app/test-compress-perf: report header
> improvement
> 
> 
> 
> > -----Original Message-----
> > From: dev <dev-bounces@dpdk.org> On Behalf Of Artur Trybula
> > Sent: Monday, June 3, 2019 3:54 PM
> > To: dev@dpdk.org; fiona.trahe@intel.com; arturx.trybula@intel.com;
> > adamx.dybkowski@intel.com
> > Subject: [dpdk-dev] [PATCH] app/test-compress-perf: report header
> > improvement
> >
> > 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.
> >
> > Signed-off-by: Artur Trybula <arturx.trybula@intel.com>
> > ---
> >  app/test-compress-perf/main.c | 105
> > +++++++++++++++++++++++++++++++---
> >  1 file changed, 98 insertions(+), 7 deletions(-)
> >
> ......
> >
> > -
> > +static void
> > +print_report_header(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("\nWarning: 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");
> 
> [Shally] Why these kind of warnings? If total_bufs > 1, then, how behavior
> would change? Same question for segment size as well?
> [Artur] I suppose you mean the second one "Warning: There is only one ...".
> It's just to preserve the structure of the report for both variants.  But I agree,
> there is nothing wrong if there is only one op.
> Please notice that each op is processed independently, in most cases it
> implies lower compression ratio. The best situation is to have all the input
> data aggregated into one op (even if partitioned across many mbufs).
> 
[Shally] Then can we better covert this in terms of "stateless" and "stateful" . Example, 
Say it like: 
Since test app support stateless mode only, thus if input data is segmented across multiple buffers (i.e. op)
Then each will be processed independently (i.e. in stateless mode) . Thus to gain better results, it might be more useful to put 
all data in one buffer.

> > +	} else
> > +		printf("Warning: something wrong happened!!\n");
> > +
> > +	printf("\nFor the current input parameters (segment size = %u,"
> > +			" segments number = %u):\n",
> > +		tests_res.segment_sz,
> > +		tests_res.segments_per_buff);
> > +	printf("	• Total number of segments: %d\n",
> > +		tests_res.total_segments);
> > +	printf("	• %u segments %u bytes long, last segment %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 segments 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);
> > +	}
> > +}
> >
> >  int
> >  main(int argc, char **argv)
> > @@ -533,8 +622,9 @@ main(int argc, char **argv)
> >  	else
> >  		level = test_data->level.list[0];
> >
> > +	print_report_header();
> > +
> [Shally] looks like we're printing input characteristics and possible
> performance behavior. Is that the intention of this API?
> [Artur] That was the idea to have a tool for verification how the data
> partitioning affects the compression performance. Your description Shally is
> very accurate: "... printing input characteristics and possible performance
> behavior" and that is the intention of this API.
[Shally] Then probably we can add this in function description.
> 
> 
> >  	printf("Burst size = %u\n", test_data->burst_sz);
> > -	printf("File size = %zu\n", test_data->input_data_sz);
> >
> >  	printf("%6s%12s%17s%19s%21s%15s%21s%23s%16s\n",
> >  		"Level", "Comp size", "Comp ratio [%]", @@ -612,3 +702,4
> @@
> > main(int argc, char **argv)
> >  	}
> >  	return ret;
> >  }
> > +
> > --
> > 2.17.1

Re: [dpdk-dev] [PATCH] app/test-compress-perf: report header improvement

[Trybula, ArturX]

-----Original Message-----
From: Shally Verma [mailto:shallyv@marvell.com] 
Sent: Thursday, June 27, 2019 13:01
To: Trybula, ArturX <arturx.trybula@intel.com>; dev@dpdk.org; Trahe, Fiona <fiona.trahe@intel.com>; Dybkowski, AdamX <adamx.dybkowski@intel.com>
Subject: RE: [dpdk-dev] [PATCH] app/test-compress-perf: report header improvement



> -----Original Message-----
> From: Trybula, ArturX <arturx.trybula@intel.com>
> Sent: Thursday, June 27, 2019 2:29 PM
> To: Shally Verma <shallyv@marvell.com>; dev@dpdk.org; Trahe, Fiona 
> <fiona.trahe@intel.com>; Dybkowski, AdamX <adamx.dybkowski@intel.com>
> Subject: [EXT] RE: [dpdk-dev] [PATCH] app/test-compress-perf: report 
> header improvement
> 
> External Email
> 
> ----------------------------------------------------------------------
> 
> 
> -----Original Message-----
> From: Shally Verma [mailto:shallyv@marvell.com]
> Sent: Wednesday, June 26, 2019 19:04
> To: Trybula, ArturX <arturx.trybula@intel.com>; dev@dpdk.org; Trahe, 
> Fiona <fiona.trahe@intel.com>; Dybkowski, AdamX 
> <adamx.dybkowski@intel.com>
> Subject: RE: [dpdk-dev] [PATCH] app/test-compress-perf: report header 
> improvement
> 
> 
> 
> > -----Original Message-----
> > From: dev <dev-bounces@dpdk.org> On Behalf Of Artur Trybula
> > Sent: Monday, June 3, 2019 3:54 PM
> > To: dev@dpdk.org; fiona.trahe@intel.com; arturx.trybula@intel.com; 
> > adamx.dybkowski@intel.com
> > Subject: [dpdk-dev] [PATCH] app/test-compress-perf: report header 
> > improvement
> >
> > 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.
> >
> > Signed-off-by: Artur Trybula <arturx.trybula@intel.com>
> > ---
> >  app/test-compress-perf/main.c | 105
> > +++++++++++++++++++++++++++++++---
> >  1 file changed, 98 insertions(+), 7 deletions(-)
> >
> ......
> >
> > -
> > +static void
> > +print_report_header(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("\nWarning: 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");
> 
> [Shally] Why these kind of warnings? If total_bufs > 1, then, how 
> behavior would change? Same question for segment size as well?
> [Artur] I suppose you mean the second one "Warning: There is only one ...".
> It's just to preserve the structure of the report for both variants.  
> But I agree, there is nothing wrong if there is only one op.
> Please notice that each op is processed independently, in most cases 
> it implies lower compression ratio. The best situation is to have all 
> the input data aggregated into one op (even if partitioned across many mbufs).
> 
[Shally] Then can we better covert this in terms of "stateless" and "stateful" . Example, Say it like: 
Since test app support stateless mode only, thus if input data is segmented across multiple buffers (i.e. op) Then each will be processed independently (i.e. in stateless mode) . Thus to gain better results, it might be more useful to put all data in one buffer.
[Artur] We started from an idea of simple checking memory allocation. From my point of view we should avoid making it too complex unless there is a clear need. This is a "simple tool" for quick verification what is the effect of the input parameters. It's for developers familiar with the compression. I agree with you that in case of supporting both stateless and stateful compression such information should be printed out.


> > +	} else
> > +		printf("Warning: something wrong happened!!\n");
> > +
> > +	printf("\nFor the current input parameters (segment size = %u,"
> > +			" segments number = %u):\n",
> > +		tests_res.segment_sz,
> > +		tests_res.segments_per_buff);
> > +	printf("	• Total number of segments: %d\n",
> > +		tests_res.total_segments);
> > +	printf("	• %u segments %u bytes long, last segment %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 segments 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);
> > +	}
> > +}
> >
> >  int
> >  main(int argc, char **argv)
> > @@ -533,8 +622,9 @@ main(int argc, char **argv)
> >  	else
> >  		level = test_data->level.list[0];
> >
> > +	print_report_header();
> > +
> [Shally] looks like we're printing input characteristics and possible 
> performance behavior. Is that the intention of this API?
> [Artur] That was the idea to have a tool for verification how the data 
> partitioning affects the compression performance. Your description 
> Shally is very accurate: "... printing input characteristics and 
> possible performance behavior" and that is the intention of this API.
[Shally] Then probably we can add this in function description.
[Artur] To be precise it is not a typical API function. It's an internal (static) function of the perf test. 
> 
> 
> >  	printf("Burst size = %u\n", test_data->burst_sz);
> > -	printf("File size = %zu\n", test_data->input_data_sz);
> >
> >  	printf("%6s%12s%17s%19s%21s%15s%21s%23s%16s\n",
> >  		"Level", "Comp size", "Comp ratio [%]", @@ -612,3 +702,4
> @@
> > main(int argc, char **argv)
> >  	}
> >  	return ret;
> >  }
> > +
> > --
> > 2.17.1

Re: [dpdk-dev] [PATCH] app/test-compress-perf: report header improvement

[Shally Verma]

> -----Original Message-----
> From: Trybula, ArturX <arturx.trybula@intel.com>
> Sent: Thursday, June 27, 2019 5:17 PM
> To: Shally Verma <shallyv@marvell.com>; dev@dpdk.org; Trahe, Fiona
> <fiona.trahe@intel.com>; Dybkowski, AdamX
> <adamx.dybkowski@intel.com>
> Subject: [EXT] RE: [dpdk-dev] [PATCH] app/test-compress-perf: report
> header improvement
> 
> External Email
> 
> ----------------------------------------------------------------------
> 
> 
> -----Original Message-----
> From: Shally Verma [mailto:shallyv@marvell.com]
> Sent: Thursday, June 27, 2019 13:01
> To: Trybula, ArturX <arturx.trybula@intel.com>; dev@dpdk.org; Trahe, Fiona
> <fiona.trahe@intel.com>; Dybkowski, AdamX
> <adamx.dybkowski@intel.com>
> Subject: RE: [dpdk-dev] [PATCH] app/test-compress-perf: report header
> improvement
> 
> 
> 
> > -----Original Message-----
> > From: Trybula, ArturX <arturx.trybula@intel.com>
> > Sent: Thursday, June 27, 2019 2:29 PM
> > To: Shally Verma <shallyv@marvell.com>; dev@dpdk.org; Trahe, Fiona
> > <fiona.trahe@intel.com>; Dybkowski, AdamX
> <adamx.dybkowski@intel.com>
> > Subject: [EXT] RE: [dpdk-dev] [PATCH] app/test-compress-perf: report
> > header improvement
> >
> > External Email
> >
> > ----------------------------------------------------------------------
> >
> >
> > -----Original Message-----
> > From: Shally Verma [mailto:shallyv@marvell.com]
> > Sent: Wednesday, June 26, 2019 19:04
> > To: Trybula, ArturX <arturx.trybula@intel.com>; dev@dpdk.org; Trahe,
> > Fiona <fiona.trahe@intel.com>; Dybkowski, AdamX
> > <adamx.dybkowski@intel.com>
> > Subject: RE: [dpdk-dev] [PATCH] app/test-compress-perf: report header
> > improvement
> >
> >
> >
> > > -----Original Message-----
> > > From: dev <dev-bounces@dpdk.org> On Behalf Of Artur Trybula
> > > Sent: Monday, June 3, 2019 3:54 PM
> > > To: dev@dpdk.org; fiona.trahe@intel.com; arturx.trybula@intel.com;
> > > adamx.dybkowski@intel.com
> > > Subject: [dpdk-dev] [PATCH] app/test-compress-perf: report header
> > > improvement
> > >
> > > 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.
> > >
> > > Signed-off-by: Artur Trybula <arturx.trybula@intel.com>
> > > ---
> > >  app/test-compress-perf/main.c | 105
> > > +++++++++++++++++++++++++++++++---
> > >  1 file changed, 98 insertions(+), 7 deletions(-)
> > >
> > ......
> > >
> > > -
> > > +static void
> > > +print_report_header(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("\nWarning: 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");
> >
> > [Shally] Why these kind of warnings? If total_bufs > 1, then, how
> > behavior would change? Same question for segment size as well?
> > [Artur] I suppose you mean the second one "Warning: There is only one
> ...".
> > It's just to preserve the structure of the report for both variants.
> > But I agree, there is nothing wrong if there is only one op.
> > Please notice that each op is processed independently, in most cases
> > it implies lower compression ratio. The best situation is to have all
> > the input data aggregated into one op (even if partitioned across many
> mbufs).
> >
> [Shally] Then can we better covert this in terms of "stateless" and "stateful" .
> Example, Say it like:
> Since test app support stateless mode only, thus if input data is segmented
> across multiple buffers (i.e. op) Then each will be processed independently
> (i.e. in stateless mode) . Thus to gain better results, it might be more useful
> to put all data in one buffer.
> [Artur] We started from an idea of simple checking memory allocation. From
> my point of view we should avoid making it too complex unless there is a
> clear need. This is a "simple tool" for quick verification what is the effect of
> the input parameters. It's for developers familiar with the compression. I
> agree with you that in case of supporting both stateless and stateful
> compression such information should be printed out.
> 
[Shally] Okay. I leave it to your comfort. But could you confirm if I understood it correct? 
I mean keep num_bufs = 1, will result in better history thus better compression.

> 
> > > +	} else
> > > +		printf("Warning: something wrong happened!!\n");
> > > +
> > > +	printf("\nFor the current input parameters (segment size = %u,"
> > > +			" segments number = %u):\n",
> > > +		tests_res.segment_sz,
> > > +		tests_res.segments_per_buff);
> > > +	printf("	• Total number of segments: %d\n",
> > > +		tests_res.total_segments);
> > > +	printf("	• %u segments %u bytes long, last segment %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 segments 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);
> > > +	}
> > > +}
> > >
> > >  int
> > >  main(int argc, char **argv)
> > > @@ -533,8 +622,9 @@ main(int argc, char **argv)
> > >  	else
> > >  		level = test_data->level.list[0];
> > >
> > > +	print_report_header();
> > > +
> > [Shally] looks like we're printing input characteristics and possible
> > performance behavior. Is that the intention of this API?
> > [Artur] That was the idea to have a tool for verification how the data
> > partitioning affects the compression performance. Your description
> > Shally is very accurate: "... printing input characteristics and
> > possible performance behavior" and that is the intention of this API.
> [Shally] Then probably we can add this in function description.
> [Artur] To be precise it is not a typical API function. It's an internal (static)
> function of the perf test.
[Shally] Oh ya. Then can we change name .. print_report_header() sounds like printing report on test results but here intent is different. Something like print_test_dynamics() seem more applicable?

> >
> >
> > >  	printf("Burst size = %u\n", test_data->burst_sz);
> > > -	printf("File size = %zu\n", test_data->input_data_sz);
> > >
> > >  	printf("%6s%12s%17s%19s%21s%15s%21s%23s%16s\n",
> > >  		"Level", "Comp size", "Comp ratio [%]", @@ -612,3 +702,4
> > @@
> > > main(int argc, char **argv)
> > >  	}
> > >  	return ret;
> > >  }
> > > +
> > > --
> > > 2.17.1

Re: [dpdk-dev] [PATCH] app/test-compress-perf: report header improvement

[Trybula, ArturX]

-----Original Message-----
From: Shally Verma [mailto:shallyv@marvell.com] 
Sent: Thursday, June 27, 2019 16:39
To: Trybula, ArturX <arturx.trybula@intel.com>; dev@dpdk.org; Trahe, Fiona <fiona.trahe@intel.com>; Dybkowski, AdamX <adamx.dybkowski@intel.com>
Subject: RE: [dpdk-dev] [PATCH] app/test-compress-perf: report header improvement



> -----Original Message-----
> From: Trybula, ArturX <arturx.trybula@intel.com>
> Sent: Thursday, June 27, 2019 5:17 PM
> To: Shally Verma <shallyv@marvell.com>; dev@dpdk.org; Trahe, Fiona 
> <fiona.trahe@intel.com>; Dybkowski, AdamX <adamx.dybkowski@intel.com>
> Subject: [EXT] RE: [dpdk-dev] [PATCH] app/test-compress-perf: report 
> header improvement
> 
> External Email
> 
> ----------------------------------------------------------------------
> 
> 
> -----Original Message-----
> From: Shally Verma [mailto:shallyv@marvell.com]
> Sent: Thursday, June 27, 2019 13:01
> To: Trybula, ArturX <arturx.trybula@intel.com>; dev@dpdk.org; Trahe, 
> Fiona <fiona.trahe@intel.com>; Dybkowski, AdamX 
> <adamx.dybkowski@intel.com>
> Subject: RE: [dpdk-dev] [PATCH] app/test-compress-perf: report header 
> improvement
> 
> 
> 
> > -----Original Message-----
> > From: Trybula, ArturX <arturx.trybula@intel.com>
> > Sent: Thursday, June 27, 2019 2:29 PM
> > To: Shally Verma <shallyv@marvell.com>; dev@dpdk.org; Trahe, Fiona 
> > <fiona.trahe@intel.com>; Dybkowski, AdamX
> <adamx.dybkowski@intel.com>
> > Subject: [EXT] RE: [dpdk-dev] [PATCH] app/test-compress-perf: report 
> > header improvement
> >
> > External Email
> >
> > --------------------------------------------------------------------
> > --
> >
> >
> > -----Original Message-----
> > From: Shally Verma [mailto:shallyv@marvell.com]
> > Sent: Wednesday, June 26, 2019 19:04
> > To: Trybula, ArturX <arturx.trybula@intel.com>; dev@dpdk.org; Trahe, 
> > Fiona <fiona.trahe@intel.com>; Dybkowski, AdamX 
> > <adamx.dybkowski@intel.com>
> > Subject: RE: [dpdk-dev] [PATCH] app/test-compress-perf: report 
> > header improvement
> >
> >
> >
> > > -----Original Message-----
> > > From: dev <dev-bounces@dpdk.org> On Behalf Of Artur Trybula
> > > Sent: Monday, June 3, 2019 3:54 PM
> > > To: dev@dpdk.org; fiona.trahe@intel.com; arturx.trybula@intel.com; 
> > > adamx.dybkowski@intel.com
> > > Subject: [dpdk-dev] [PATCH] app/test-compress-perf: report header 
> > > improvement
> > >
> > > 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.
> > >
> > > Signed-off-by: Artur Trybula <arturx.trybula@intel.com>
> > > ---
> > >  app/test-compress-perf/main.c | 105
> > > +++++++++++++++++++++++++++++++---
> > >  1 file changed, 98 insertions(+), 7 deletions(-)
> > >
> > ......
> > >
> > > -
> > > +static void
> > > +print_report_header(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("\nWarning: 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");
> >
> > [Shally] Why these kind of warnings? If total_bufs > 1, then, how 
> > behavior would change? Same question for segment size as well?
> > [Artur] I suppose you mean the second one "Warning: There is only 
> > one
> ...".
> > It's just to preserve the structure of the report for both variants.
> > But I agree, there is nothing wrong if there is only one op.
> > Please notice that each op is processed independently, in most cases 
> > it implies lower compression ratio. The best situation is to have 
> > all the input data aggregated into one op (even if partitioned 
> > across many
> mbufs).
> >
> [Shally] Then can we better covert this in terms of "stateless" and "stateful" .
> Example, Say it like:
> Since test app support stateless mode only, thus if input data is 
> segmented across multiple buffers (i.e. op) Then each will be 
> processed independently (i.e. in stateless mode) . Thus to gain better 
> results, it might be more useful to put all data in one buffer.
> [Artur] We started from an idea of simple checking memory allocation. 
> From my point of view we should avoid making it too complex unless 
> there is a clear need. This is a "simple tool" for quick verification 
> what is the effect of the input parameters. It's for developers 
> familiar with the compression. I agree with you that in case of 
> supporting both stateless and stateful compression such information should be printed out.
> 
[Shally] Okay. I leave it to your comfort. But could you confirm if I understood it correct? 
I mean keep num_bufs = 1, will result in better history thus better compression.
[Artur] Only stateless compression is implemented. That’s why there is a difference when data is segmented over many ops (num_mufs > 1). In case of stateful compression history really helps to improve the compression ratio.

> 
> > > +	} else
> > > +		printf("Warning: something wrong happened!!\n");
> > > +
> > > +	printf("\nFor the current input parameters (segment size = %u,"
> > > +			" segments number = %u):\n",
> > > +		tests_res.segment_sz,
> > > +		tests_res.segments_per_buff);
> > > +	printf("	• Total number of segments: %d\n",
> > > +		tests_res.total_segments);
> > > +	printf("	• %u segments %u bytes long, last segment %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 segments 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);
> > > +	}
> > > +}
> > >
> > >  int
> > >  main(int argc, char **argv)
> > > @@ -533,8 +622,9 @@ main(int argc, char **argv)
> > >  	else
> > >  		level = test_data->level.list[0];
> > >
> > > +	print_report_header();
> > > +
> > [Shally] looks like we're printing input characteristics and 
> > possible performance behavior. Is that the intention of this API?
> > [Artur] That was the idea to have a tool for verification how the 
> > data partitioning affects the compression performance. Your 
> > description Shally is very accurate: "... printing input 
> > characteristics and possible performance behavior" and that is the intention of this API.
> [Shally] Then probably we can add this in function description.
> [Artur] To be precise it is not a typical API function. It's an 
> internal (static) function of the perf test.
[Shally] Oh ya. Then can we change name .. print_report_header() sounds like printing report on test results but here intent is different. Something like print_test_dynamics() seem more applicable?
[Artur] Both names are ok for me. Depends on your point of view. Ok, I will change it for you.

> >
> >
> > >  	printf("Burst size = %u\n", test_data->burst_sz);
> > > -	printf("File size = %zu\n", test_data->input_data_sz);
> > >
> > >  	printf("%6s%12s%17s%19s%21s%15s%21s%23s%16s\n",
> > >  		"Level", "Comp size", "Comp ratio [%]", @@ -612,3 +702,4
> > @@
> > > main(int argc, char **argv)
> > >  	}
> > >  	return ret;
> > >  }
> > > +
> > > --
> > > 2.17.1

[dpdk-dev] [PATCH v2 0/1] add extra features to test-compress-perf

[Artur Trybula]

v2 changes:

	- merged with the series "add multiple cores feature to
		test-compress-perf"
	- function renamed: print_report_header() -> print_test_dynamics()

This patch depends on the series "add multiple cores feature to
test-compress-perf"
https://patches.dpdk.org/cover/56024/

Signed-off-by: Artur Trybula <arturx.trybula@intel.com>
---

Artur Trybula (1):
  app/test-compress-perf: report header improvement

 .../comp_perf_test_common.c                   | 94 ++++++++++++++++++-
 .../comp_perf_test_common.h                   |  3 +
 app/test-compress-perf/main.c                 |  4 +-
 3 files changed, 98 insertions(+), 3 deletions(-)

-- 
2.17.1

[dpdk-dev] [PATCH v2 1/1] app/test-compress-perf: report header improvement

[Artur Trybula]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=y, Size: 6265 bytes --]

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.

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

diff --git a/app/test-compress-perf/comp_perf_test_common.c b/app/test-compress-perf/comp_perf_test_common.c
index dc9d0b0f4..dc3eaccac 100644
--- a/app/test-compress-perf/comp_perf_test_common.c
+++ b/app/test-compress-perf/comp_perf_test_common.c
@@ -15,6 +15,19 @@
 
 #define DIV_CEIL(a, b)  ((a) / (b) + ((a) % (b) != 0))
 
+struct performance_tests_results {
+	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 performance_tests_results tests_res;
+//extern static struct performance_tests_results tests_res;
+
 int
 param_range_check(uint16_t size, const struct rte_param_log2_range *range)
 {
@@ -169,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;
 }
 
@@ -177,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 */
@@ -203,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
@@ -280,5 +301,74 @@ 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("\nWarning: 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,"
+			" segments number = %u):\n",
+		tests_res.segment_sz,
+		tests_res.segments_per_buff);
+	printf("	• Total number of segments: %d\n",
+		tests_res.total_segments);
+	printf("	• %u segments %u bytes long, last segment %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 segments 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);
+	}
+}
diff --git a/app/test-compress-perf/comp_perf_test_common.h b/app/test-compress-perf/comp_perf_test_common.h
index 9c11e3a00..c72f3ef3c 100644
--- a/app/test-compress-perf/comp_perf_test_common.h
+++ b/app/test-compress-perf/comp_perf_test_common.h
@@ -38,4 +38,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..76961bffb 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();
+
 	while (test_data->level <= test_data->level_lst.max) {
 
 		i = 0;
-- 
2.17.1

Re: [dpdk-dev] [PATCH v2 1/1] app/test-compress-perf: report header improvement

[Jerin Jacob Kollanukkaran]

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Artur Trybula
> Sent: Friday, July 5, 2019 12:00 PM
> To: dev@dpdk.org; fiona.trahe@intel.com;
> shally.verma@caviumnetworks.com; adamx.dybkowski@intel.com;
> arturx.trybula@intel.com; akhil.goyal@nxp.com
> Subject: [dpdk-dev] [PATCH v2 1/1] app/test-compress-perf: report header
> improvement
> 
> 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.
> 
> Signed-off-by: Artur Trybula <arturx.trybula@intel.com>
> ---
>  .../comp_perf_test_common.c                   | 94 ++++++++++++++++++-
>  .../comp_perf_test_common.h                   |  3 +
>  app/test-compress-perf/main.c                 |  4 +-
>  3 files changed, 98 insertions(+), 3 deletions(-)
> 
> diff --git a/app/test-compress-perf/comp_perf_test_common.c b/app/test-
> compress-perf/comp_perf_test_common.c
> index dc9d0b0f4..dc3eaccac 100644
> --- a/app/test-compress-perf/comp_perf_test_common.c
> +++ b/app/test-compress-perf/comp_perf_test_common.c
> @@ -15,6 +15,19 @@
> 
>  #define DIV_CEIL(a, b)  ((a) / (b) + ((a) % (b) != 0))
> 
> +struct performance_tests_results {
> +	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 performance_tests_results tests_res; //extern static


Spotted C++ style commented unused code.

> +struct performance_tests_results tests_res;

[dpdk-dev] [PATCH v3 0/1] add extra features to test-compress-perf

[Artur Trybula]

v2 changes:

        - merged with the series "add multiple cores feature to
                test-compress-perf"
        - function renamed: print_report_header() -> print_test_dynamics()

v3 changes:

        - commented code removed

This patch depends on the series "add multiple cores feature to
test-compress-perf"
https://patches.dpdk.org/cover/56024/

Signed-off-by: Artur Trybula <arturx.trybula@intel.com>
---

Artur Trybula (1):
  app/test-compress-perf: report header improvement

 .../comp_perf_test_common.c                   | 93 ++++++++++++++++++-
 .../comp_perf_test_common.h                   |  3 +
 app/test-compress-perf/main.c                 |  4 +-
 3 files changed, 97 insertions(+), 3 deletions(-)

-- 
2.17.1

[dpdk-dev] [PATCH v3 1/1] app/test-compress-perf: report header improvement

[Artur Trybula]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=y, Size: 6204 bytes --]

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.

Signed-off-by: Artur Trybula <arturx.trybula@intel.com>
---
 .../comp_perf_test_common.c                   | 93 ++++++++++++++++++-
 .../comp_perf_test_common.h                   |  3 +
 app/test-compress-perf/main.c                 |  4 +-
 3 files changed, 97 insertions(+), 3 deletions(-)

diff --git a/app/test-compress-perf/comp_perf_test_common.c b/app/test-compress-perf/comp_perf_test_common.c
index dc9d0b0f4..7c5f36939 100644
--- a/app/test-compress-perf/comp_perf_test_common.c
+++ b/app/test-compress-perf/comp_perf_test_common.c
@@ -15,6 +15,18 @@
 
 #define DIV_CEIL(a, b)  ((a) / (b) + ((a) % (b) != 0))
 
+struct performance_tests_results {
+	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 performance_tests_results tests_res;
+
 int
 param_range_check(uint16_t size, const struct rte_param_log2_range *range)
 {
@@ -169,6 +181,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;
 }
 
@@ -177,9 +196,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 */
@@ -203,7 +223,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
@@ -280,5 +300,74 @@ 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("\nWarning: 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,"
+			" segments number = %u):\n",
+		tests_res.segment_sz,
+		tests_res.segments_per_buff);
+	printf("	• Total number of segments: %d\n",
+		tests_res.total_segments);
+	printf("	• %u segments %u bytes long, last segment %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 segments 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);
+	}
+}
diff --git a/app/test-compress-perf/comp_perf_test_common.h b/app/test-compress-perf/comp_perf_test_common.h
index 9c11e3a00..c72f3ef3c 100644
--- a/app/test-compress-perf/comp_perf_test_common.h
+++ b/app/test-compress-perf/comp_perf_test_common.h
@@ -38,4 +38,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..76961bffb 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();
+
 	while (test_data->level <= test_data->level_lst.max) {
 
 		i = 0;
-- 
2.17.1

[dpdk-dev] [PATCH v4 0/1] add extra features to test-compress-perf

[Artur Trybula]

v4 changes:
        - information about compression threads printed on screen

v3 changes:
        - commented code removed

v2 changes:
        - merged with the series "add multiple cores feature to
                test-compress-perf"
        - function renamed: print_report_header() -> print_test_dynamics()

This patch depends on the series "add multiple cores feature to
test-compress-perf"
https://patches.dpdk.org/cover/56236/

Signed-off-by: Artur Trybula <arturx.trybula@intel.com>

Artur Trybula (1):
  app/test-compress-perf: report header improvement

 .../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(-)

-- 
2.17.1

[dpdk-dev] [PATCH v4 1/1] app/test-compress-perf: report header improvement

[Artur Trybula]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=y, Size: 8017 bytes --]

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(-)

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..3dc9349b0 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 performance_tests_results {
+	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 performance_tests_results 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("\nWarning: 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,"
+			" segments number = %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;
-- 
2.17.1

Re: [dpdk-dev] [EXT] [PATCH v4 1/1] app/test-compress-perf: report header improvement

[Shally Verma]

> -----Original Message-----
> From: Artur Trybula <arturx.trybula@intel.com>
> Sent: Friday, July 12, 2019 4:13 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 v4 1/1] app/test-compress-perf: report header
> improvement
> 
> External Email
> 
> ----------------------------------------------------------------------
> 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(-)
> 
> 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))
Question: runner() will be executed by each core so is ctx is thread local variable or global.? If it can be made thread local, then there would be no need of atomic here?

> +		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..3dc9349b0 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 performance_tests_results {
> +	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;
> +};
These looks more like test configuration than result. If you agree, then can rename it to test_config .. 

> +
> +static struct performance_tests_results 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;
>
Minimum segment per mbuf is 1. Then why don’t we initialize it to 1 here?
 
>  	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"
A comma after 'input parameters' would improve readability here

> +				" 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("\nWarning: there is only one op with %u segments –"
May "Warning:" be replaced with "Info: " here.

> +				" 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,"
> +			" segments number = %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);
> +	}
Probably this if and else if here can be replaced by just 1 statement.


> +	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;
> --
> 2.17.1

Re: [dpdk-dev] [EXT] [PATCH v4 1/1] app/test-compress-perf: report header improvement

[Trybula, ArturX]

-----Original Message-----
From: Shally Verma [mailto:shallyv@marvell.com] 
Sent: Monday, July 15, 2019 14:47
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 v4 1/1] app/test-compress-perf: report header improvement



> -----Original Message-----
> From: Artur Trybula <arturx.trybula@intel.com>
> Sent: Friday, July 12, 2019 4:13 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 v4 1/1] app/test-compress-perf: report header 
> improvement
> 
> External Email
> 
> ----------------------------------------------------------------------
> 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(-)
> 
> 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))
Question: runner() will be executed by each core so is ctx is thread local variable or global.? If it can be made thread local, then there would be no need of atomic here?
[Artur] Runners are restarted for each compression level. It's enough to have the data printed only 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..3dc9349b0 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 performance_tests_results {
> +	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;
> +};
These looks more like test configuration than result. If you agree, then can rename it to test_config .. 
[Artur] These are exactly tests results, internal variables used by the algorithm. A kind of snapshot. Config by definition is used for configuration. In this case all the fields are only printed out.

> +
> +static struct performance_tests_results 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;
>
Minimum segment per mbuf is 1. Then why don’t we initialize it to 1 here?
[Artur] You are right Shally, if everything is fine with the input params and with the algorithm, but it should be left 0 in case of problems with mbufs allocation (current implementation).

 
>  	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"
A comma after 'input parameters' would improve readability here
[Artur] - Ok

> +				" 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("\nWarning: there is only one op with %u segments –"
May "Warning:" be replaced with "Info: " here.
[Artur] Ok

> +				" 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,"
> +			" segments number = %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);
> +	}
Probably this if and else if here can be replaced by just 1 statement.
[Artur] I think it's ok. This version is clear. If you like I can remove {} brackets from the "else if".


> +	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;
> --
> 2.17.1

Re: [dpdk-dev] [EXT] [PATCH v4 1/1] app/test-compress-perf: report header improvement

[Trahe, Fiona]

Hi Artur, Shally,

> -----Original Message-----
> From: Trybula, ArturX
> Sent: Monday, July 15, 2019 3:01 PM
> To: Shally Verma <shallyv@marvell.com>; dev@dpdk.org; Trahe, Fiona <fiona.trahe@intel.com>;
> Dybkowski, AdamX <adamx.dybkowski@intel.com>; akhil.goyal@nxp.com
> Subject: RE: [EXT] [PATCH v4 1/1] app/test-compress-perf: report header improvement
> 
> -----Original Message-----
> From: Shally Verma [mailto:shallyv@marvell.com]
> Sent: Monday, July 15, 2019 14:47
> 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 v4 1/1] app/test-compress-perf: report header improvement
> 
> 
> 
> > -----Original Message-----
> > From: Artur Trybula <arturx.trybula@intel.com>
> > Sent: Friday, July 12, 2019 4:13 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 v4 1/1] app/test-compress-perf: report header
> > improvement
> >
> > External Email
> >
> > ----------------------------------------------------------------------
> > 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(-)
> >
> > 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))
> Question: runner() will be executed by each core so is ctx is thread local variable or global.? If it can be
> made thread local, then there would be no need of atomic here?
> [Artur] Runners are restarted for each compression level. It's enough to have the data printed only
> once.
[Fiona] added clarification - we originally did as you suggest, but in cases where there are multiple params, e.g. level 1-9, the thread is started/stopped for each level, so the header was printing out many times - always with the same data. So we needed the atomic to resolve.

> 
> > +		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..3dc9349b0 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 performance_tests_results {
> > +	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;
> > +};
> These looks more like test configuration than result. If you agree, then can rename it to test_config ..
> [Artur] These are exactly tests results, internal variables used by the algorithm. A kind of snapshot.
> Config by definition is used for configuration. In this case all the fields are only printed out.
[Fiona] I can see the confusion, it's not the input config - but it's also confusing to call it results, as that implies the throughput and ratio numbers. How about cperf_buffer_info?
> 
> > +
> > +static struct performance_tests_results 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;
> >
> Minimum segment per mbuf is 1. Then why don’t we initialize it to 1 here?
> [Artur] You are right Shally, if everything is fine with the input params and with the algorithm, but it
> should be left 0 in case of problems with mbufs allocation (current implementation).
> 
> 
> >  	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"
> A comma after 'input parameters' would improve readability here
> [Artur] - Ok
> 
> > +				" 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("\nWarning: there is only one op with %u segments –"
> May "Warning:" be replaced with "Info: " here.
> [Artur] Ok
> 
> > +				" 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,"
> > +			" segments number = %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);
> > +	}
> Probably this if and else if here can be replaced by just 1 statement.
> [Artur] I think it's ok. This version is clear. If you like I can remove {} brackets from the "else if".
[Fiona] I think it's ok as is - there are different output whether >1 or ==1, so better to leave as separate paths.
> 
> 
> > +	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;
> > --
> > 2.17.1

Re: [dpdk-dev] [EXT] [PATCH v4 1/1] app/test-compress-perf: report header improvement

[Shally Verma]

> -----Original Message-----
> From: Trahe, Fiona <fiona.trahe@intel.com>
> Sent: Tuesday, July 16, 2019 7:20 PM
> To: Trybula, ArturX <arturx.trybula@intel.com>; Shally Verma
> <shallyv@marvell.com>; dev@dpdk.org; Dybkowski, AdamX
> <adamx.dybkowski@intel.com>; akhil.goyal@nxp.com
> Cc: Trahe, Fiona <fiona.trahe@intel.com>
> Subject: RE: [EXT] [PATCH v4 1/1] app/test-compress-perf: report header
> improvement
> 
....

> >
> > > -----Original Message-----
> > > From: Artur Trybula <arturx.trybula@intel.com>
> > > Sent: Friday, July 12, 2019 4:13 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 v4 1/1] app/test-compress-perf: report header
> > > improvement
> > >
> > > External Email
> > >
> > > --------------------------------------------------------------------
> > > -- 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(-)
> > >
> > > 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))
> > Question: runner() will be executed by each core so is ctx is thread
> > local variable or global.? If it can be made thread local, then there would be
> no need of atomic here?
> > [Artur] Runners are restarted for each compression level. It's enough
> > to have the data printed only once.
> [Fiona] added clarification - we originally did as you suggest, but in cases
> where there are multiple params, e.g. level 1-9, the thread is
> started/stopped for each level, so the header was printing out many times -
> always with the same data. So we needed the atomic to resolve.
> 
[Shally] Okay. 

> >
....

> > > diff --git a/app/test-compress-perf/comp_perf_test_common.c
> > > b/app/test- compress-perf/comp_perf_test_common.c
> > > index 472c76686..3dc9349b0 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 performance_tests_results {
> > > +	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;
> > > +};
> > These looks more like test configuration than result. If you agree, then can
> rename it to test_config ..
> > [Artur] These are exactly tests results, internal variables used by the
> algorithm. A kind of snapshot.
> > Config by definition is used for configuration. In this case all the fields are
> only printed out.
> [Fiona] I can see the confusion, it's not the input config - but it's also
> confusing to call it results, as that implies the throughput and ratio numbers.
> How about cperf_buffer_info?
[Shally] this sound better.

> >
...

> > > +	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);
> > > +	}
> > Probably this if and else if here can be replaced by just 1 statement.
> > [Artur] I think it's ok. This version is clear. If you like I can remove {}
> brackets from the "else if".
> [Fiona] I think it's ok as is - there are different output whether >1 or ==1, so
> better to leave as separate paths.
[Shally] Okay. That was just my 2 cents. Am fine it "as is".

> >
..
> > > 2.17.1

[dpdk-dev] [PATCH v5 0/1] add extra features to test-compress-perf

[Artur Trybula]

v5 changes:
        - some minor changes (rewording, structure renaming, etc.)

v4 changes:
        - information about compression threads printed on screen

v3 changes:
        - commented code removed

v2 changes:
        - merged with the series "add multiple cores feature to
                test-compress-perf"
        - function renamed: print_report_header() -> print_test_dynamics()

This patch depends on the series "add multiple cores feature to
test-compress-perf"
https://patches.dpdk.org/cover/56236/

Signed-off-by: Artur Trybula <arturx.trybula@intel.com>

Artur Trybula (1):
  app/test-compress-perf: report header improvement

 .../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(-)

-- 
2.17.1

[dpdk-dev] [PATCH v5 1/1] app/test-compress-perf: report header improvement

[Artur Trybula]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=y, Size: 8008 bytes --]

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(-)

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;
-- 
2.17.1

Re: [dpdk-dev] [EXT] [PATCH v5 1/1] app/test-compress-perf: report header improvement

[Shally Verma]

> -----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.

....

Re: [dpdk-dev] [EXT] [PATCH v5 1/1] app/test-compress-perf: report header improvement

[Trybula, ArturX]

-----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.

....

Re: [dpdk-dev] [EXT] [PATCH v5 1/1] app/test-compress-perf: report header improvement

[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

Thanks.

Re: [dpdk-dev] [EXT] [PATCH v5 1/1] app/test-compress-perf: report header improvement

[Thomas Monjalon]

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.

Re: [dpdk-dev] [EXT] [PATCH v5 1/1] app/test-compress-perf: report header improvement

[Trahe, Fiona]

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.

[dpdk-dev] [PATCH v6 0/1] add extra features to test-compress-perf

[Adam Dybkowski]

v6 changes:
        - replace special bullet characters with normal asterisks

v5 changes:
        - some minor changes (rewording, structure renaming, etc.)

v4 changes:
        - information about compression threads printed on screen

v3 changes:
        - commented code removed

v2 changes:
        - merged with the series "add multiple cores feature to
                test-compress-perf"
        - function renamed: print_report_header() -> print_test_dynamics()

Signed-off-by: Adam Dybkowski <adamx.dybkowski@intel.com>
Signed-off-by: Artur Trybula <arturx.trybula@intel.com>

Artur Trybula (1):
  app/test-compress-perf: provide more detailed report

 .../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(-)

-- 
2.17.1

[dpdk-dev] [PATCH v6 1/1] app/test-compress-perf: provide more detailed report

[Adam Dybkowski]

From: Artur Trybula <arturx.trybula@intel.com>

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.
Information about threads, cores, devices and queue-pairs
is also printed.

Signed-off-by: Artur Trybula <arturx.trybula@intel.com>
Signed-off-by: Adam Dybkowski <adamx.dybkowski@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(-)

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..bfecbf166 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 buffer_info;
+
 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;
 	}
+
+	buffer_info.total_segments = total_segs;
+	buffer_info.segment_sz = test_data->seg_sz;
+	buffer_info.total_buffs = mem->total_bufs;
+	buffer_info.segments_per_buff = test_data->max_sgl_segs;
+	buffer_info.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)
 		}
 	}
 
+	buffer_info.segments_per_last_buff = segs_per_mbuf;
+	buffer_info.last_segment_sz = data_sz;
+
 	return 0;
 }
+
+void
+print_test_dynamics(void)
+{
+	uint32_t opt_total_segs = DIV_CEIL(buffer_info.input_data_sz,
+			MAX_SEG_SIZE);
+
+	if (buffer_info.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 (buffer_info.total_buffs == 1) {
+		printf("\nInfo: there is only one op with %u segments –"
+				" the compression ratio is the best.\n",
+			buffer_info.segments_per_last_buff);
+		if (buffer_info.segment_sz < MAX_SEG_SIZE)
+			printf("To reduce compression time, please use"
+					" bigger segment size: %d.\n",
+				MAX_SEG_SIZE);
+		else if (buffer_info.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",
+		buffer_info.segment_sz,
+		buffer_info.segments_per_buff);
+	printf("	* Total number of buffers: %d\n",
+		buffer_info.total_segments);
+	printf("	* %u buffer(s) %u bytes long, last buffer %u"
+			" byte(s) long\n",
+		buffer_info.total_segments - 1,
+		buffer_info.segment_sz,
+		buffer_info.last_segment_sz);
+	printf("	* Number of ops: %u\n", buffer_info.total_buffs);
+	printf("	* Total memory allocation: %u\n",
+		(buffer_info.total_segments - 1) * buffer_info.segment_sz
+		+ buffer_info.last_segment_sz);
+	if (buffer_info.total_buffs > 1)
+		printf("	* %u ops: %u segment(s) in each,"
+				" segment size %u\n",
+			buffer_info.total_buffs - 1,
+			buffer_info.segments_per_buff,
+			buffer_info.segment_sz);
+	if (buffer_info.segments_per_last_buff > 1) {
+		printf("	* 1 op %u segments:\n",
+				buffer_info.segments_per_last_buff);
+		printf("		o %u segment size %u\n",
+			buffer_info.segments_per_last_buff - 1,
+			buffer_info.segment_sz);
+		printf("		o last segment size %u\n",
+			buffer_info.last_segment_sz);
+	} else if (buffer_info.segments_per_last_buff == 1) {
+		printf("	* 1 op (the last one): %u segment %u"
+				" byte(s) long\n\n",
+			buffer_info.segments_per_last_buff,
+			buffer_info.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;
-- 
2.17.1

[dpdk-dev] [PATCH v7 0/1] add extra features to test-compress-perf

[Adam Dybkowski]

v7 changes:
        - one more special character replaced with a minus sign

v6 changes:
        - replace special bullet characters with normal asterisks

v5 changes:
        - some minor changes (rewording, structure renaming, etc.)

v4 changes:
        - information about compression threads printed on screen

v3 changes:
        - commented code removed

v2 changes:
        - merged with the series "add multiple cores feature to
                test-compress-perf"
        - function renamed: print_report_header() -> print_test_dynamics()

Signed-off-by: Adam Dybkowski <adamx.dybkowski@intel.com>
Signed-off-by: Artur Trybula <arturx.trybula@intel.com>

Artur Trybula (1):
  app/test-compress-perf: provide more detailed report

 .../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(-)

-- 
2.17.1

[dpdk-dev] [PATCH v7 1/1] app/test-compress-perf: provide more detailed report

[Adam Dybkowski]

From: Artur Trybula <arturx.trybula@intel.com>

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.
Information about threads, cores, devices and queue-pairs
is also printed.

Signed-off-by: Artur Trybula <arturx.trybula@intel.com>
Signed-off-by: Adam Dybkowski <adamx.dybkowski@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(-)

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..28be9f257 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 buffer_info;
+
 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;
 	}
+
+	buffer_info.total_segments = total_segs;
+	buffer_info.segment_sz = test_data->seg_sz;
+	buffer_info.total_buffs = mem->total_bufs;
+	buffer_info.segments_per_buff = test_data->max_sgl_segs;
+	buffer_info.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)
 		}
 	}
 
+	buffer_info.segments_per_last_buff = segs_per_mbuf;
+	buffer_info.last_segment_sz = data_sz;
+
 	return 0;
 }
+
+void
+print_test_dynamics(void)
+{
+	uint32_t opt_total_segs = DIV_CEIL(buffer_info.input_data_sz,
+			MAX_SEG_SIZE);
+
+	if (buffer_info.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 (buffer_info.total_buffs == 1) {
+		printf("\nInfo: there is only one op with %u segments -"
+				" the compression ratio is the best.\n",
+			buffer_info.segments_per_last_buff);
+		if (buffer_info.segment_sz < MAX_SEG_SIZE)
+			printf("To reduce compression time, please use"
+					" bigger segment size: %d.\n",
+				MAX_SEG_SIZE);
+		else if (buffer_info.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",
+		buffer_info.segment_sz,
+		buffer_info.segments_per_buff);
+	printf("	* Total number of buffers: %d\n",
+		buffer_info.total_segments);
+	printf("	* %u buffer(s) %u bytes long, last buffer %u"
+			" byte(s) long\n",
+		buffer_info.total_segments - 1,
+		buffer_info.segment_sz,
+		buffer_info.last_segment_sz);
+	printf("	* Number of ops: %u\n", buffer_info.total_buffs);
+	printf("	* Total memory allocation: %u\n",
+		(buffer_info.total_segments - 1) * buffer_info.segment_sz
+		+ buffer_info.last_segment_sz);
+	if (buffer_info.total_buffs > 1)
+		printf("	* %u ops: %u segment(s) in each,"
+				" segment size %u\n",
+			buffer_info.total_buffs - 1,
+			buffer_info.segments_per_buff,
+			buffer_info.segment_sz);
+	if (buffer_info.segments_per_last_buff > 1) {
+		printf("	* 1 op %u segments:\n",
+				buffer_info.segments_per_last_buff);
+		printf("		o %u segment size %u\n",
+			buffer_info.segments_per_last_buff - 1,
+			buffer_info.segment_sz);
+		printf("		o last segment size %u\n",
+			buffer_info.last_segment_sz);
+	} else if (buffer_info.segments_per_last_buff == 1) {
+		printf("	* 1 op (the last one): %u segment %u"
+				" byte(s) long\n\n",
+			buffer_info.segments_per_last_buff,
+			buffer_info.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;
-- 
2.17.1

Re: [dpdk-dev] [PATCH v7 1/1] app/test-compress-perf: provide more detailed report

[Trahe, Fiona]

> -----Original Message-----
> From: Dybkowski, AdamX
> Sent: Wednesday, July 24, 2019 2:55 PM
> To: dev@dpdk.org; Trahe, Fiona <fiona.trahe@intel.com>
> Cc: Trybula, ArturX <arturx.trybula@intel.com>; Dybkowski, AdamX <adamx.dybkowski@intel.com>
> Subject: [PATCH v7 1/1] app/test-compress-perf: provide more detailed report
> 
> From: Artur Trybula <arturx.trybula@intel.com>
> 
> 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.
> Information about threads, cores, devices and queue-pairs
> is also printed.
> 
> Signed-off-by: Artur Trybula <arturx.trybula@intel.com>
> Signed-off-by: Adam Dybkowski <adamx.dybkowski@intel.com>
Acked-by: Fiona Trahe <fiona.trahe@intel.com>

Re: [dpdk-dev] [PATCH v7 1/1] app/test-compress-perf: provide more detailed report

[Akhil Goyal]

> >
> > From: Artur Trybula <arturx.trybula@intel.com>
> >
> > 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.
> > Information about threads, cores, devices and queue-pairs
> > is also printed.
> >
> > Signed-off-by: Artur Trybula <arturx.trybula@intel.com>
> > Signed-off-by: Adam Dybkowski <adamx.dybkowski@intel.com>
> Acked-by: Fiona Trahe <fiona.trahe@intel.com>

Applied to dpdk-next-crypto

Thanks.

Re: [dpdk-dev] [EXT] [PATCH v5 0/1] add extra features to test-compress-perf

[Shally Verma]

> -----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 0/1] add extra features to test-compress-perf
> 
> External Email
> 
> ----------------------------------------------------------------------
> v5 changes:
>         - some minor changes (rewording, structure renaming, etc.)
> 
> v4 changes:
>         - information about compression threads printed on screen
> 
> v3 changes:
>         - commented code removed
> 
> v2 changes:
>         - merged with the series "add multiple cores feature to
>                 test-compress-perf"
>         - function renamed: print_report_header() -> print_test_dynamics()
> 
> This patch depends on the series "add multiple cores feature to test-
> compress-perf"
> https://patches.dpdk.org/cover/56236/
> 
> Signed-off-by: Artur Trybula <arturx.trybula@intel.com>
> 
Other than minor change suggested. Series 
Acked -by : Shally Verma <shallyv@marvell.com>

> Artur Trybula (1):
>   app/test-compress-perf: report header improvement
> 
>  .../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(-)
> 
> --
> 2.17.1