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 E6431A0351; Mon, 10 Jan 2022 08:27:01 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 748F8411DB; Mon, 10 Jan 2022 08:27:01 +0100 (CET) Received: from mail-io1-f46.google.com (mail-io1-f46.google.com [209.85.166.46]) by mails.dpdk.org (Postfix) with ESMTP id 71B9F4013F for ; Mon, 10 Jan 2022 08:27:00 +0100 (CET) Received: by mail-io1-f46.google.com with SMTP id u8so16345192iol.5 for ; Sun, 09 Jan 2022 23:27:00 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=hIH4i1a/U7600gBJxgjgWQgtEhlur/whAQoNblZH1VY=; b=UovvkD0NpLRwd43DQF4m5yR4Hesr2I9nomu8ClUSen7fe4bV90TUU9IFRlnM1uhaGj qDuxVlvq3816zN9OxTSvHRarvBWX+cSXDAWrN2TzuuyXq8MrBkLzcPI0uo+ZDYnFfV+Q WswWkgsTbQuOzaAt0WbKgMN8bO6vM9BFLe2U7PPQw6VRLSRYxdBtjSEe3hdlpgxkCRuP Q0uRqYVJeVKyViUQZzMtWtkCEv0AN8UmvSMz9/E+oxvkC7e1TQQ+p0irsGVJiEMDP4Do z9xAoRN0BtVEaMWLPZw7Vf5M9F2+Ke93csSHtEpDAqAQQpV+5wJ2shHv1JdzwRVKEjsD AqWg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=hIH4i1a/U7600gBJxgjgWQgtEhlur/whAQoNblZH1VY=; b=K1kq0FabkXDsLjzljCaJ4gt049bYQpf09kCrlPQAdqBdCgaAVbHT7lEVegytv8If6L QIi+XRUkm5GVhtRf7BsRX2nn9NTugjNEg11KDDBENGv8aPFYjqhfDYqxk2F3fdwcKDMf /zRwRFWdFqGePCSav0wuEaFO1yHw63ir86s4xJ8LlQ5UYTKpbxYSPbZbRt8+qg2Y6Gfs dgepqgNUjGwBYo5sby5CU4wAtgGvOmtkmnLRZEyWSeoy4mXa/0sUC42+glRGC5VCv2fI NzvWniEFoV/GRpwsmApDnW35H+pi2jq8Cc9Sfa7foAyca5XwzP06907jiQcaoRcODWBM hR1w== X-Gm-Message-State: AOAM531cUUchIOKbkwrHWr5w4UFSP63opwB/Cxgguev9ykzSFIwROo1s IUQjoLZ2gCr7ddjpxCb/4k9dP9M0VVxmQ8XStsffEYtZ X-Google-Smtp-Source: ABdhPJzN2Ul7oQaK91zF3TgLjZ+v0iRjSx+O5NJOHKDMBxA0MhUklvui0RaKn60pfZmbecnAQrU9wM0Xyz0N9andxz0= X-Received: by 2002:a05:6638:260b:: with SMTP id m11mr35422193jat.280.1641799619743; Sun, 09 Jan 2022 23:26:59 -0800 (PST) MIME-Version: 1.0 References: <98CBD80474FA8B44BF855DF32C47DC35D86DB2@smartserver.smartshare.dk> <20220106122345.82740-1-mb@smartsharesystems.com> <98CBD80474FA8B44BF855DF32C47DC35D86DE7@smartserver.smartshare.dk> In-Reply-To: <98CBD80474FA8B44BF855DF32C47DC35D86DE7@smartserver.smartshare.dk> From: Jerin Jacob Date: Mon, 10 Jan 2022 12:56:33 +0530 Message-ID: Subject: Re: [PATCH] mempool: optimize incomplete cache handling To: =?UTF-8?Q?Morten_Br=C3=B8rup?= Cc: Olivier Matz , Andrew Rybchenko , dpdk-dev Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable 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 On Fri, Jan 7, 2022 at 2:16 PM Morten Br=C3=B8rup wrote: > > > From: Jerin Jacob [mailto:jerinjacobk@gmail.com] > > Sent: Thursday, 6 January 2022 17.55 > > > > On Thu, Jan 6, 2022 at 5:54 PM Morten Br=C3=B8rup > > wrote: > > > > > > A flush threshold for the mempool cache was introduced in DPDK > > version > > > 1.3, but rte_mempool_do_generic_get() was not completely updated back > > > then. > > > > > > The incompleteness did not cause any functional bugs, so this patch > > > could be considered refactoring for the purpose of cleaning up. > > > > > > This patch completes the update of rte_mempool_do_generic_get() as > > > follows: > > > > > > 1. A few comments were malplaced or no longer correct. > > > Some comments have been updated/added/corrected. > > > > > > 2. The code that initially screens the cache request was not updated. > > > The initial screening compared the request length to the cache size, > > > which was correct before, but became irrelevant with the introduction > > of > > > the flush threshold. E.g. the cache can hold up to flushthresh > > objects, > > > which is more than its size, so some requests were not served from > > the > > > cache, even though they could be. > > > The initial screening has now been corrected to match the initial > > > screening in rte_mempool_do_generic_put(), which verifies that a > > cache > > > is present, and that the length of the request does not overflow the > > > memory allocated for the cache. > > > > > > 3. The code flow for satisfying the request from the cache was weird. > > > The likely code path where the objects are simply served from the > > cache > > > was treated as unlikely; now it is treated as likely. > > > And in the code path where the cache was backfilled first, numbers > > were > > > added and subtracted from the cache length; now this code path simply > > > sets the cache length to its final value. > > > > > > 4. The objects were returned in reverse order. > > > Returning the objects in reverse order is not necessary, so > > rte_memcpy() > > > is now used instead. > > > > Have you checked the performance with network workload? > > IMO, reverse order makes sense(LIFO vs FIFO). > > The LIFO makes the cache warm as the same buffers are reused > > frequently. > > I have not done any performance testing. We probably agree that the only = major difference lies in how the objects are returned. And we probably also= agree that rte_memcpy() is faster than the copy loop it replaced, especial= ly when n is constant at compile time. So the performance difference mainly= depends on the application, which I will discuss below. > > Let's first consider LIFO vs. FIFO. > > The key argument for the rte_memcpy() optimization is that we are still g= etting the burst of objects from the top of the stack (LIFO); only the orde= r of the objects inside the burst is not reverse anymore. > > Here is an example: > > The cache initially contains 8 objects: 01234567. > > 8 more objects are put into the cache: 89ABCDEF. > > The cache now holds: 0123456789ABCDEF. Agree. However I think, it may matter with less sized L1 cache machines and burst size is more where it plays role what can be in L1 with the scheme. I would suggest splitting each performance improvement as a separate patch for better tracking and quantity of the performance improvement. I think, mempool performance test and tx only stream mode in testpmd can quantify patches. > > Getting 4 objects from the cache gives us CDEF instead of FEDC, i.e. we a= re still getting the 4 objects most recently put into the cache. > > Furthermore, if the application is working with fixed size bursts, it wil= l usually put and get the same size burst, i.e. put the burst 89ABCDEF into= the cache, and then get the burst 89ABCDEF from the cache again. > > > Here is an example unfavorable scenario: > > The cache initially contains 4 objects, which have gone cold: 0123. > > 4 more objects, which happen to be hot, are put into the cache: 4567. > > Getting 8 objects from the cache gives us 01234567 instead of 76543210. > > Now, if the application only processes the first 4 of the 8 objects in th= e burst, it would have benefitted from those objects being the hot 7654 obj= ects instead of the cold 0123 objects. > > However, I think that most applications process the complete burst, so I = do consider this scenario unlikely. > > Similarly, a pipelined application doesn't process objects in reverse ord= er at every other step in the pipeline, even though the previous step in th= e pipeline most recently touched the last object of the burst. > > > My overall conclusion was that the benefit of using rte_memcpy() outweigh= s the disadvantage of the unfavorable scenario, because I consider the prob= ability of the unfavorable scenario occurring very low. But again: it mainl= y depends on the application. > > If anyone disagrees with the risk analysis described above, I will happil= y provide a version 2 of the patch, where the objects are still returned in= reverse order. After all, the rte_memcpy() benefit is relatively small com= pared to the impact if the unlikely scenario occurs. >