* [PATCH] app/dma-perf: add average latency per worker @ 2023-12-19 16:40 Vipin Varghese 2024-02-23 9:45 ` fengchengwen 0 siblings, 1 reply; 6+ messages in thread From: Vipin Varghese @ 2023-12-19 16:40 UTC (permalink / raw) To: dev, stable, honest.jiang, gmuthukrishn, ferruh.yigit Modify the user display data with total average latency per worker. Signed-off-by: Vipin Varghese <vipin.varghese@amd.com> --- app/test-dma-perf/benchmark.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/test-dma-perf/benchmark.c b/app/test-dma-perf/benchmark.c index 9b1f58c78c..8b6886af62 100644 --- a/app/test-dma-perf/benchmark.c +++ b/app/test-dma-perf/benchmark.c @@ -470,7 +470,8 @@ mem_copy_benchmark(struct test_configure *cfg, bool is_dma) bandwidth_total += bandwidth; avg_cycles_total += avg_cycles; } - printf("\nTotal Bandwidth: %.3lf Gbps, Total MOps: %.3lf\n", bandwidth_total, mops_total); + printf("\nAverage Cycles/op: %.2lf, Total Bandwidth: %.3lf Gbps, Total MOps: %.3lf\n", + (float) avg_cycles_total / nb_workers, bandwidth_total, mops_total); snprintf(output_str[MAX_WORKER_NB], MAX_OUTPUT_STR_LEN, CSV_TOTAL_LINE_FMT, cfg->scenario_id, nr_buf, memory * nb_workers, avg_cycles_total / nb_workers, bandwidth_total, mops_total); -- 2.34.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] app/dma-perf: add average latency per worker 2023-12-19 16:40 [PATCH] app/dma-perf: add average latency per worker Vipin Varghese @ 2024-02-23 9:45 ` fengchengwen 2024-02-27 9:50 ` Varghese, Vipin 2024-02-28 3:05 ` Varghese, Vipin 0 siblings, 2 replies; 6+ messages in thread From: fengchengwen @ 2024-02-23 9:45 UTC (permalink / raw) To: Vipin Varghese, dev, stable, honest.jiang, gmuthukrishn, ferruh.yigit Hi Vipin, On 2023/12/20 0:40, Vipin Varghese wrote: > Modify the user display data with total average latency per worker. > > Signed-off-by: Vipin Varghese <vipin.varghese@amd.com> > --- > app/test-dma-perf/benchmark.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/app/test-dma-perf/benchmark.c b/app/test-dma-perf/benchmark.c > index 9b1f58c78c..8b6886af62 100644 > --- a/app/test-dma-perf/benchmark.c > +++ b/app/test-dma-perf/benchmark.c > @@ -470,7 +470,8 @@ mem_copy_benchmark(struct test_configure *cfg, bool is_dma) > bandwidth_total += bandwidth; > avg_cycles_total += avg_cycles; > } > - printf("\nTotal Bandwidth: %.3lf Gbps, Total MOps: %.3lf\n", bandwidth_total, mops_total); > + printf("\nAverage Cycles/op: %.2lf, Total Bandwidth: %.3lf Gbps, Total MOps: %.3lf\n", > + (float) avg_cycles_total / nb_workers, bandwidth_total, mops_total); Because this is total stats, suggest add Total prefix, e.g. "Total Average Cycles/op" I think print format keep one-digit precision is enough. Also please modify CSV_TOTAL_LINE_FMT make sure the csv also have same precision of Cycles/op. Thanks > snprintf(output_str[MAX_WORKER_NB], MAX_OUTPUT_STR_LEN, CSV_TOTAL_LINE_FMT, > cfg->scenario_id, nr_buf, memory * nb_workers, > avg_cycles_total / nb_workers, bandwidth_total, mops_total); > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] app/dma-perf: add average latency per worker 2024-02-23 9:45 ` fengchengwen @ 2024-02-27 9:50 ` Varghese, Vipin 2024-02-27 13:09 ` fengchengwen 2024-02-28 3:05 ` Varghese, Vipin 1 sibling, 1 reply; 6+ messages in thread From: Varghese, Vipin @ 2024-02-27 9:50 UTC (permalink / raw) To: fengchengwen, dev, stable, honest.jiang, gmuthukrishn, ferruh.yigit On 2/23/2024 3:15 PM, fengchengwen wrote: > Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding. > > > Hi Vipin, > > On 2023/12/20 0:40, Vipin Varghese wrote: >> Modify the user display data with total average latency per worker. >> >> Signed-off-by: Vipin Varghese <vipin.varghese@amd.com> >> --- >> app/test-dma-perf/benchmark.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/app/test-dma-perf/benchmark.c b/app/test-dma-perf/benchmark.c >> index 9b1f58c78c..8b6886af62 100644 >> --- a/app/test-dma-perf/benchmark.c >> +++ b/app/test-dma-perf/benchmark.c >> @@ -470,7 +470,8 @@ mem_copy_benchmark(struct test_configure *cfg, bool is_dma) >> bandwidth_total += bandwidth; >> avg_cycles_total += avg_cycles; >> } >> - printf("\nTotal Bandwidth: %.3lf Gbps, Total MOps: %.3lf\n", bandwidth_total, mops_total); >> + printf("\nAverage Cycles/op: %.2lf, Total Bandwidth: %.3lf Gbps, Total MOps: %.3lf\n", >> + (float) avg_cycles_total / nb_workers, bandwidth_total, mops_total); thanks for the suggestions, please find my observations below > Because this is total stats, suggest add Total prefix, e.g. "Total Average Cycles/op" I did not follow this, so please let me try to explain my understanding. For `n` operation we count the average cycles, then we add the cycles to form `total average cycles`; this is then divide by `n` operations. Making this per operation what is the average cycles taken for the round trip time. Hence `Total Average Cyeles/op` does not sound right, but `Average Cycles / op` does. > I think print format keep one-digit precision is enough. Also please modify CSV_TOTAL_LINE_FMT > make sure the csv also have same precision of Cycles/op. We have checked the CSV formatting a find 1. the precision for average cycle/op is 2 digits precision. 2. already the CVS format has average cycles/op integrated. Hence no change is required. > > Thanks > >> snprintf(output_str[MAX_WORKER_NB], MAX_OUTPUT_STR_LEN, CSV_TOTAL_LINE_FMT, >> cfg->scenario_id, nr_buf, memory * nb_workers, >> avg_cycles_total / nb_workers, bandwidth_total, mops_total); >> ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] app/dma-perf: add average latency per worker 2024-02-27 9:50 ` Varghese, Vipin @ 2024-02-27 13:09 ` fengchengwen 2024-02-28 3:06 ` Varghese, Vipin 0 siblings, 1 reply; 6+ messages in thread From: fengchengwen @ 2024-02-27 13:09 UTC (permalink / raw) To: Varghese, Vipin, dev, stable, honest.jiang, gmuthukrishn, ferruh.yigit Hi Vipin, On 2024/2/27 17:50, Varghese, Vipin wrote: > > On 2/23/2024 3:15 PM, fengchengwen wrote: >> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding. >> >> >> Hi Vipin, >> >> On 2023/12/20 0:40, Vipin Varghese wrote: >>> Modify the user display data with total average latency per worker. >>> >>> Signed-off-by: Vipin Varghese <vipin.varghese@amd.com> >>> --- >>> app/test-dma-perf/benchmark.c | 3 ++- >>> 1 file changed, 2 insertions(+), 1 deletion(-) >>> >>> diff --git a/app/test-dma-perf/benchmark.c b/app/test-dma-perf/benchmark.c >>> index 9b1f58c78c..8b6886af62 100644 >>> --- a/app/test-dma-perf/benchmark.c >>> +++ b/app/test-dma-perf/benchmark.c >>> @@ -470,7 +470,8 @@ mem_copy_benchmark(struct test_configure *cfg, bool is_dma) >>> bandwidth_total += bandwidth; >>> avg_cycles_total += avg_cycles; >>> } >>> - printf("\nTotal Bandwidth: %.3lf Gbps, Total MOps: %.3lf\n", bandwidth_total, mops_total); >>> + printf("\nAverage Cycles/op: %.2lf, Total Bandwidth: %.3lf Gbps, Total MOps: %.3lf\n", >>> + (float) avg_cycles_total / nb_workers, bandwidth_total, mops_total); > > thanks for the suggestions, please find my observations below > >> Because this is total stats, suggest add Total prefix, e.g. "Total Average Cycles/op" > I did not follow this, so please let me try to explain my understanding. For `n` operation we count the average cycles, then > we add the cycles to form `total average cycles`; this is then divide by `n` operations. Making this per operation what is the > average cycles taken for the round trip time. Hence `Total Average Cyeles/op` does not sound right, but `Average Cycles / op` does. OK My intention was to differentiate (since they have the same beginning). >> I think print format keep one-digit precision is enough. Also please modify CSV_TOTAL_LINE_FMT >> make sure the csv also have same precision of Cycles/op. > > We have checked the CSV formatting a find > 1. the precision for average cycle/op is 2 digits precision. > 2. already the CVS format has average cycles/op integrated. > > Hence no change is required. It's weird. We can see more clear when add together: #define CSV_TOTAL_LINE_FMT "Scenario %u Summary, , , , , ,%u,%.2lf,%u,%.3lf,%.3lf\n" snprintf(output_str[MAX_WORKER_NB], MAX_OUTPUT_STR_LEN, CSV_TOTAL_LINE_FMT, cfg->scenario_id, nr_buf, memory * nb_workers, avg_cycles_total / nb_workers, bandwidth_total, mops_total); The bandwidth_total, mops_total both are float, which take last two "%.31f", then "%u" is for "avg_cycles_total / nb_workers" Also, avg_cycles_total / nb_workers both are u32, and its result is u32 if not with force convert. You could modify with "avg_cycles_total*1.0 / nb_workers", then compile will output warning: [2035/3727] Compiling C object app/dpdk-test-dma-perf.p/test-dma-perf_benchmark.c.o ../../dpdk/app/test-dma-perf/benchmark.c: In function ‘mem_copy_benchmark’: ../../dpdk/app/test-dma-perf/benchmark.c:26:28: warning: format ‘%u’ expects argument of type ‘unsigned int’, but argument 7 has type ‘double’ [-Wformat=] #define CSV_TOTAL_LINE_FMT "Scenario %u Summary, , , , , ,%u,%.2lf,%u,%.3lf,%.3lf\n" Thanks ^ > >> >> Thanks >> >>> snprintf(output_str[MAX_WORKER_NB], MAX_OUTPUT_STR_LEN, CSV_TOTAL_LINE_FMT, >>> cfg->scenario_id, nr_buf, memory * nb_workers, >>> avg_cycles_total / nb_workers, bandwidth_total, mops_total); >>> > . ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] app/dma-perf: add average latency per worker 2024-02-27 13:09 ` fengchengwen @ 2024-02-28 3:06 ` Varghese, Vipin 0 siblings, 0 replies; 6+ messages in thread From: Varghese, Vipin @ 2024-02-28 3:06 UTC (permalink / raw) To: fengchengwen, dev, stable, honest.jiang, gmuthukrishn, ferruh.yigit On 2/27/2024 6:39 PM, fengchengwen wrote: > Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding. > > > Hi Vipin, > > On 2024/2/27 17:50, Varghese, Vipin wrote: >> On 2/23/2024 3:15 PM, fengchengwen wrote: >>> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding. >>> >>> >>> Hi Vipin, >>> >>> On 2023/12/20 0:40, Vipin Varghese wrote: >>>> Modify the user display data with total average latency per worker. >>>> >>>> Signed-off-by: Vipin Varghese <vipin.varghese@amd.com> >>>> --- >>>> app/test-dma-perf/benchmark.c | 3 ++- >>>> 1 file changed, 2 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/app/test-dma-perf/benchmark.c b/app/test-dma-perf/benchmark.c >>>> index 9b1f58c78c..8b6886af62 100644 >>>> --- a/app/test-dma-perf/benchmark.c >>>> +++ b/app/test-dma-perf/benchmark.c >>>> @@ -470,7 +470,8 @@ mem_copy_benchmark(struct test_configure *cfg, bool is_dma) >>>> bandwidth_total += bandwidth; >>>> avg_cycles_total += avg_cycles; >>>> } >>>> - printf("\nTotal Bandwidth: %.3lf Gbps, Total MOps: %.3lf\n", bandwidth_total, mops_total); >>>> + printf("\nAverage Cycles/op: %.2lf, Total Bandwidth: %.3lf Gbps, Total MOps: %.3lf\n", >>>> + (float) avg_cycles_total / nb_workers, bandwidth_total, mops_total); >> thanks for the suggestions, please find my observations below >> >>> Because this is total stats, suggest add Total prefix, e.g. "Total Average Cycles/op" >> I did not follow this, so please let me try to explain my understanding. For `n` operation we count the average cycles, then >> we add the cycles to form `total average cycles`; this is then divide by `n` operations. Making this per operation what is the >> average cycles taken for the round trip time. Hence `Total Average Cyeles/op` does not sound right, but `Average Cycles / op` does. > OK > My intention was to differentiate (since they have the same beginning). > >>> I think print format keep one-digit precision is enough. Also please modify CSV_TOTAL_LINE_FMT >>> make sure the csv also have same precision of Cycles/op. >> We have checked the CSV formatting a find >> 1. the precision for average cycle/op is 2 digits precision. >> 2. already the CVS format has average cycles/op integrated. >> >> Hence no change is required. > It's weird. > > We can see more clear when add together: > #define CSV_TOTAL_LINE_FMT "Scenario %u Summary, , , , , ,%u,%.2lf,%u,%.3lf,%.3lf\n" > snprintf(output_str[MAX_WORKER_NB], MAX_OUTPUT_STR_LEN, CSV_TOTAL_LINE_FMT, > cfg->scenario_id, nr_buf, memory * nb_workers, > avg_cycles_total / nb_workers, bandwidth_total, mops_total); > The bandwidth_total, mops_total both are float, which take last two "%.31f", then "%u" is for "avg_cycles_total / nb_workers" > > Also, avg_cycles_total / nb_workers both are u32, and its result is u32 if not with force convert. > > You could modify with "avg_cycles_total*1.0 / nb_workers", then compile will output warning: > > [2035/3727] Compiling C object app/dpdk-test-dma-perf.p/test-dma-perf_benchmark.c.o > ../../dpdk/app/test-dma-perf/benchmark.c: In function ‘mem_copy_benchmark’: > ../../dpdk/app/test-dma-perf/benchmark.c:26:28: warning: format ‘%u’ expects argument of type ‘unsigned int’, but argument 7 has type ‘double’ [-Wformat=] > #define CSV_TOTAL_LINE_FMT "Scenario %u Summary, , , , , ,%u,%.2lf,%u,%.3lf,%.3lf\n" thank you Chengwen, let me share v2 for this patch with both print and csv in same format. > > Thanks > ^ >>> Thanks >>> >>>> snprintf(output_str[MAX_WORKER_NB], MAX_OUTPUT_STR_LEN, CSV_TOTAL_LINE_FMT, >>>> cfg->scenario_id, nr_buf, memory * nb_workers, >>>> avg_cycles_total / nb_workers, bandwidth_total, mops_total); >>>> >> . ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] app/dma-perf: add average latency per worker 2024-02-23 9:45 ` fengchengwen 2024-02-27 9:50 ` Varghese, Vipin @ 2024-02-28 3:05 ` Varghese, Vipin 1 sibling, 0 replies; 6+ messages in thread From: Varghese, Vipin @ 2024-02-28 3:05 UTC (permalink / raw) To: fengchengwen, dev, stable, honest.jiang, gmuthukrishn, ferruh.yigit On 2/23/2024 3:15 PM, fengchengwen wrote: > Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding. > > > Hi Vipin, > > On 2023/12/20 0:40, Vipin Varghese wrote: >> Modify the user display data with total average latency per worker. >> >> Signed-off-by: Vipin Varghese <vipin.varghese@amd.com> >> --- >> app/test-dma-perf/benchmark.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/app/test-dma-perf/benchmark.c b/app/test-dma-perf/benchmark.c >> index 9b1f58c78c..8b6886af62 100644 >> --- a/app/test-dma-perf/benchmark.c >> +++ b/app/test-dma-perf/benchmark.c >> @@ -470,7 +470,8 @@ mem_copy_benchmark(struct test_configure *cfg, bool is_dma) >> bandwidth_total += bandwidth; >> avg_cycles_total += avg_cycles; >> } >> - printf("\nTotal Bandwidth: %.3lf Gbps, Total MOps: %.3lf\n", bandwidth_total, mops_total); >> + printf("\nAverage Cycles/op: %.2lf, Total Bandwidth: %.3lf Gbps, Total MOps: %.3lf\n", >> + (float) avg_cycles_total / nb_workers, bandwidth_total, mops_total); > Because this is total stats, suggest add Total prefix, e.g. "Total Average Cycles/op" > > I think print format keep one-digit precision is enough. Also please modify CSV_TOTAL_LINE_FMT > make sure the csv also have same precision of Cycles/op. Thank you Chengwen, let me share v2 shortly. > > Thanks > >> snprintf(output_str[MAX_WORKER_NB], MAX_OUTPUT_STR_LEN, CSV_TOTAL_LINE_FMT, >> cfg->scenario_id, nr_buf, memory * nb_workers, >> avg_cycles_total / nb_workers, bandwidth_total, mops_total); >> ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-02-28 3:06 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-12-19 16:40 [PATCH] app/dma-perf: add average latency per worker Vipin Varghese 2024-02-23 9:45 ` fengchengwen 2024-02-27 9:50 ` Varghese, Vipin 2024-02-27 13:09 ` fengchengwen 2024-02-28 3:06 ` Varghese, Vipin 2024-02-28 3:05 ` Varghese, Vipin
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).