From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from proxy.6wind.com (host.76.145.23.62.rev.coltfrance.com [62.23.145.76]) by dpdk.org (Postfix) with ESMTP id A83A829C6 for ; Wed, 29 Jun 2016 16:31:28 +0200 (CEST) Received: from [10.16.0.195] (unknown [10.16.0.195]) by proxy.6wind.com (Postfix) with ESMTP id 62A0A26BB6; Wed, 29 Jun 2016 16:31:28 +0200 (CEST) To: "Hunt, David" , dev@dpdk.org References: <1462472982-49782-1-git-send-email-david.hunt@intel.com> <1463669335-30378-1-git-send-email-david.hunt@intel.com> <1463669335-30378-2-git-send-email-david.hunt@intel.com> <5742FDA6.5070108@6wind.com> <576406B2.5060605@intel.com> <5767A6A0.8000800@6wind.com> <5767E8AB.1040109@intel.com> From: Olivier MATZ Message-ID: <5773DBC0.5050709@6wind.com> Date: Wed, 29 Jun 2016 16:31:28 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Icedove/38.6.0 MIME-Version: 1.0 In-Reply-To: <5767E8AB.1040109@intel.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v2 1/3] mempool: add stack (lifo) mempool handler X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 29 Jun 2016 14:31:28 -0000 Hi Dave, On 06/20/2016 02:59 PM, Hunt, David wrote: > Hi Olivier, > > On 20/6/2016 9:17 AM, Olivier Matz wrote: >> Hi David, >> >> On 06/17/2016 04:18 PM, Hunt, David wrote: >>>> After reading it, I realize that it's nearly exactly the same code than >>>> in "app/test: test external mempool handler". >>>> http://patchwork.dpdk.org/dev/patchwork/patch/12896/ >>>> >>>> We should drop one of them. If this stack handler is really useful for >>>> a performance use-case, it could go in librte_mempool. At the first >>>> read, the code looks like a demo example : it uses a simple spinlock >>>> for >>>> concurrent accesses to the common pool. Maybe the mempool cache hides >>>> this cost, in this case we could also consider removing the use of the >>>> rte_ring. >>> While I agree that the code is similar, the handler in the test is a >>> ring based handler, >>> where as this patch adds an array based handler. >> Not sure I'm getting what you are saying. Do you mean stack instead >> of array? > > Yes, apologies, stack. > >> Actually, both are stacks when talking about bulks of objects. If we >> consider each objects one by one, that's true the order will differ. >> But as discussed in [1], the cache code already reverses the order of >> objects when doing a mempool_get(). I'd say the reversing in cache code >> is not really needed (only the order of object bulks should remain the >> same). A rte_memcpy() looks to be faster, but it would require to do >> some real-life tests to validate or unvalidate this theory. >> >> So to conclude, I still think both code in app/test and lib/mempool are >> quite similar, and only one of them should be kept. >> >> [1] http://www.dpdk.org/ml/archives/dev/2016-May/039873.html > > OK, so we will probably remove the test app portion in the future is if > is not needed, > and if we apply the stack handler proposed in this patch set. Back on this thread. Maybe I misunderstood what you were saying here (because I see this comment is not addressed in v3). I don't think we should add similar code at two different places and then remove it later in another patchset. I feel it's better to have only one instance of the stack handler, either in test, or in librte_mempool. If you plan to do a v4, I think this is something that could go in 16.07-rc2. Regards, Olivier