From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <adrien.mazarguil@6wind.com>
Received: from mail-wm0-f68.google.com (mail-wm0-f68.google.com [74.125.82.68])
 by dpdk.org (Postfix) with ESMTP id B66061BA03
 for <dev@dpdk.org>; Wed, 25 Oct 2017 18:50:04 +0200 (CEST)
Received: by mail-wm0-f68.google.com with SMTP id q124so3126616wmb.0
 for <dev@dpdk.org>; Wed, 25 Oct 2017 09:50:04 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=6wind-com.20150623.gappssmtp.com; s=20150623;
 h=date:from:to:cc:subject:message-id:references:mime-version
 :content-disposition:in-reply-to;
 bh=P9+M1+L12Vb7miM9vfU829ZXxTk8xdKqSduaC8hB0gU=;
 b=ifocQE1s3piUMFbE9z4EhRvS+zwBt4Uj1Q8vjSDUA2R0Vz+hVLNMVYB8a/dWgei1+3
 7uHKJBlVuIiGb0/rborbQc15atqWdGiW3dRBN0ys4x5L/85/YWN9MBqjWoQU56mSzwdw
 f0w16K27GJbylWeoaEapY/R5oa1/v7OPj5hEvKDGd1hBQG95HFKgV9lwKMFAue0SgK7K
 EfWXj6edIwCjMSZ36T04MrQUbuLVzzYXFhZEA+vJnWHgjmYrPWeJhDO+4W1TnGlvR5tL
 +A94aenjW6mA4aRJ7hSGeAIefMv0dj66x6cUiKiu4HJAOKalmOcRiGX6h/evKLKecFVC
 BoGg==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=1e100.net; s=20161025;
 h=x-gm-message-state:date:from:to:cc:subject:message-id:references
 :mime-version:content-disposition:in-reply-to;
 bh=P9+M1+L12Vb7miM9vfU829ZXxTk8xdKqSduaC8hB0gU=;
 b=nfnKm7qYsmzmyXpo148JL6zmajKiwpgN6Q35gbPKI26s0bhcJpCE31Z2orK/TE4n76
 tkCSFaihlQeXvuUk4OepZjwEPSwmT49G3aYMVL8YOui6rPpGxzZJ2b8VurqyaHVBelil
 MSG268Y5/F/WTU4eiwlWsIK9ctltSraydo4oxy2uuGO7lmK/JtbXp5pp7ifooH236YOG
 Li+I3PjjAnnhFzZeZ0qY+bR9uWUxZAE8MCKu5c6d0b1gSrzM3wXUKH969TNLRI4cISY2
 YJsqs0thVk9NNIXLQdyZmJ0WLMAjiZj3XswKmj7UTm2S1eOm+rv8fgFz9nPZyzXwLAhO
 v1RQ==
X-Gm-Message-State: AMCzsaXi1qIjwuclSwDMfuWX6TBtloIqh1aPnzvSS4vQsk02b0eye4vm
 7aEd20H/ERJevShhkNe+D4hMjA==
X-Google-Smtp-Source: ABhQp+Ri4q+mm4xxLbk9MVLLYHTA84BtLvUm1vtmjZww2jXu8flxnfbe4bvpJ4nvklNvg93ugYVyFw==
X-Received: by 10.28.111.206 with SMTP id c75mr2465590wmi.123.1508950204459;
 Wed, 25 Oct 2017 09:50:04 -0700 (PDT)
Received: from 6wind.com (host.78.145.23.62.rev.coltfrance.com. [62.23.145.78])
 by smtp.gmail.com with ESMTPSA id v7sm2566360wrd.56.2017.10.25.09.50.03
 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);
 Wed, 25 Oct 2017 09:50:03 -0700 (PDT)
Date: Wed, 25 Oct 2017 18:49:52 +0200
From: Adrien Mazarguil <adrien.mazarguil@6wind.com>
To: Ophir Munk <ophirmu@mellanox.com>
Cc: dev@dpdk.org, Thomas Monjalon <thomas@monjalon.net>,
 Olga Shern <olgas@mellanox.com>, Matan Azrad <matan@mellanox.com>
Message-ID: <20171025164952.GI26782@6wind.com>
References: <1508752838-30408-1-git-send-email-ophirmu@mellanox.com>
 <1508768520-4810-1-git-send-email-ophirmu@mellanox.com>
 <1508768520-4810-6-git-send-email-ophirmu@mellanox.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1508768520-4810-6-git-send-email-ophirmu@mellanox.com>
Subject: Re: [dpdk-dev] [PATCH v2 5/7] net/mlx4: remove unnecessary
	variables in Tx burst
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Wed, 25 Oct 2017 16:50:04 -0000

Hi Ophir/Matan,

On Mon, Oct 23, 2017 at 02:21:58PM +0000, Ophir Munk wrote:
> From: Matan Azrad <matan@mellanox.com>
> 
> Remove usage of variables which doesn't add new information for
> performance improvement.
> 
> Signed-off-by: Matan Azrad <matan@mellanox.com>

I'm almost 100% sure this commit wasn't validated for performance on its
own. Don't mention "performance improvement" in that case.

If you're removing a couple of local variables for readability, just say
so.

More below.

> ---
>  drivers/net/mlx4/mlx4_rxtx.c | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/mlx4/mlx4_rxtx.c b/drivers/net/mlx4/mlx4_rxtx.c
> index 014a6d3..e8d9a35 100644
> --- a/drivers/net/mlx4/mlx4_rxtx.c
> +++ b/drivers/net/mlx4/mlx4_rxtx.c
> @@ -285,8 +285,6 @@ mlx4_tx_burst(void *dpdk_txq, struct rte_mbuf **pkts, uint16_t pkts_n)
>  	struct txq *txq = (struct txq *)dpdk_txq;
>  	unsigned int elts_head = txq->elts_head;
>  	const unsigned int elts_n = txq->elts_n;
> -	unsigned int elts_comp = 0;
> -	unsigned int bytes_sent = 0;
>  	unsigned int i;
>  	unsigned int max;
>  	struct mlx4_sq *sq = &txq->msq;
> @@ -498,8 +496,7 @@ mlx4_tx_burst(void *dpdk_txq, struct rte_mbuf **pkts, uint16_t pkts_n)
>  						       MLX4_BIT_WQE_OWN : 0));
>  		sq->head += nr_txbbs;
>  		elt->buf = buf;
> -		bytes_sent += buf->pkt_len;
> -		++elts_comp;
> +		txq->stats.obytes += buf->pkt_len;
>  		elts_head = elts_head_next;
>  	}
>  	/* Take a shortcut if nothing must be sent. */
> @@ -507,13 +504,12 @@ mlx4_tx_burst(void *dpdk_txq, struct rte_mbuf **pkts, uint16_t pkts_n)
>  		return 0;
>  	/* Increment send statistics counters. */
>  	txq->stats.opackets += i;
> -	txq->stats.obytes += bytes_sent;
>  	/* Make sure that descriptors are written before doorbell record. */
>  	rte_wmb();
>  	/* Ring QP doorbell. */
>  	rte_write32(txq->msq.doorbell_qpn, txq->msq.db);
>  	txq->elts_head = elts_head;
> -	txq->elts_comp += elts_comp;
> +	txq->elts_comp += i;
>  	return i;
>  }

For bytes_sent, reading these changes and assuming -O0 with the compiler
attempting to convert the code without reordering/improving things, this
replaces register variables used in a loop with memory operations on a large
structure through a pointer (txq->stats update for every iteration instead
of once at the end).

Are you really sure this is more optimized that way? Although the compiler
likely does it already with -O3, helping it avoid unnecessary memory writes
is good in my opinion.

OK for the removal of redundant elts_comp though. Although I'm pretty sure
once again the compiler didn't wait for this patch to optimize it away.

-- 
Adrien Mazarguil
6WIND