From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id DA25C43C06; Tue, 27 Feb 2024 14:09:44 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 3E00C402DE; Tue, 27 Feb 2024 14:09:44 +0100 (CET) Received: from szxga02-in.huawei.com (szxga02-in.huawei.com [45.249.212.188]) by mails.dpdk.org (Postfix) with ESMTP id AE16740150; Tue, 27 Feb 2024 14:09:41 +0100 (CET) Received: from mail.maildlp.com (unknown [172.19.163.174]) by szxga02-in.huawei.com (SkyGuard) with ESMTP id 4Tkd982rwyzLqcr; Tue, 27 Feb 2024 21:09:00 +0800 (CST) Received: from dggpeml500024.china.huawei.com (unknown [7.185.36.10]) by mail.maildlp.com (Postfix) with ESMTPS id 3DE3D140499; Tue, 27 Feb 2024 21:09:39 +0800 (CST) Received: from [10.67.121.161] (10.67.121.161) by dggpeml500024.china.huawei.com (7.185.36.10) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2507.35; Tue, 27 Feb 2024 21:09:39 +0800 Subject: Re: [PATCH] app/dma-perf: add average latency per worker To: "Varghese, Vipin" , , , , , References: <20231219164021.858-1-vipin.varghese@amd.com> <9acefd53-f617-28c1-1d6b-21b137d336eb@huawei.com> From: fengchengwen Message-ID: <53b56a35-175f-de95-0f5f-cb7a4835607c@huawei.com> Date: Tue, 27 Feb 2024 21:09:38 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.11.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8" Content-Language: en-US Content-Transfer-Encoding: 8bit X-Originating-IP: [10.67.121.161] X-ClientProxiedBy: dggems704-chm.china.huawei.com (10.3.19.181) To dggpeml500024.china.huawei.com (7.185.36.10) X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org 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 >>> --- >>>   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); >>> > .