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 1D67BA0C4E; Fri, 15 Oct 2021 15:48:19 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 07918411CB; Fri, 15 Oct 2021 15:48:19 +0200 (CEST) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mails.dpdk.org (Postfix) with ESMTP id A5B7C410F1 for ; Fri, 15 Oct 2021 15:48:17 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1634305697; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=gTjNN3F/HseBl+BIqtXMvsC+JJmG7iwqlvTVfmuPg+A=; b=A06RABFFgAndwYq+zCOOn1X/Z50tz7rdU1nJ14v42M3w1OERnoWcKuYKGjsscNKKAcMy65 L2emb5lCTfGjlQhWMX9IJ1p4jiYsLQztLyCYyW7WnneN6sJxYry8bBPSThjMmgqzaz5BY2 bUfbZfkUMRljd7Vh4ohYIOATlhtQQCQ= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-224-GPHRgaVtOqqde1aaPxSbhA-1; Fri, 15 Oct 2021 09:48:16 -0400 X-MC-Unique: GPHRgaVtOqqde1aaPxSbhA-1 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id B487C11295B2; Fri, 15 Oct 2021 13:47:59 +0000 (UTC) Received: from RHTPC1VM0NT (unknown [10.22.16.245]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 8F71260C0F; Fri, 15 Oct 2021 13:47:58 +0000 (UTC) From: Aaron Conole To: Dmitry Kozlyuk Cc: "dev@dpdk.org" , Slava Ovsiienko , Anatoly Burakov , David Marchand References: <20210921081632.858873-1-dkozlyuk@nvidia.com> <20211011085644.2716490-1-dkozlyuk@nvidia.com> <20211011085644.2716490-4-dkozlyuk@nvidia.com> Date: Fri, 15 Oct 2021 09:47:56 -0400 In-Reply-To: (Dmitry Kozlyuk's message of "Tue, 12 Oct 2021 14:48:11 +0000") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.2 (gnu/linux) MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=aconole@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain Subject: Re: [dpdk-dev] [PATCH v6 3/3] app/test: add allocator performance autotest 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 Sender: "dev" Dmitry Kozlyuk writes: >> This isn't really a test, imho. There are no assert()s. How does a developer who >> tries to fix a bug in this area know what is acceptable? >> >> Please switch the printf()s to RTE_LOG calls, and add some RTE_TEST_ASSERT >> calls to enforce some time range at the least. >> Otherwise this test will not really be checking the performance - just giving a >> report somewhere. > > I just followed DPDK naming convention of test_xxx_perf.c / xxx_perf_autotest. > They all should really be called benchmarks. Agreed - they are not really tests and it makes me wonder why we label them as such. It will be confusing. A developer who runs the perf test suite will just see "OK" everywhere and assume that all the tests are working - even if they introduce a performance regression. Maybe it would make sense to relabel them (perf-benchmark or something), so that there isn't an expectation that we have PASS / FAIL. That's a larger scope than this patch, though. > They help developers to see how the code changes affect performance. > I don't understand how this "perf test" is not in line with existing ones > and where it should properly reside. > > I'm not totally opposed to replacing printf() with RTE_LOG(), but all other test use printf(). > The drawback of the change is inconsistency, what is the benefit? RTE_LOG is captured in other places as well. printf() depending on how the test app is run might not go anywhere. Also, at least the ipsec perf test starts introducing RTE_LOG() calls - although even there they use printf() for reports. I guess it's very confusing to call all of these as 'test' since they aren't. But that's an aside, and I guess this is consistent with existing _perf.c files. >> Also, I don't understand the way the memset test works here. You do one large >> memset at the very beginning and then extrapolate the time it would take. Does >> that hold any value or should we do a memset in each iteration and enforce a >> scaled time? > > As explained above, we don't need to enforce anything, we want a report. > I've never seen a case with one NUMA node where memset() time would not scale linearly, > but benchmarks should be precise so I'll change it to memset()'ing the allocated area, thanks.