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 B824843BDF; Fri, 8 Mar 2024 09:27:33 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 83B3942EF6; Fri, 8 Mar 2024 09:27:33 +0100 (CET) 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 27ADA42EF3 for ; Fri, 8 Mar 2024 09:27:32 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1709886451; 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: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=/tjSf3tXAxvCIt4Px1vaAqYpZ/2jCPC4l0Wy/FpoZBM=; b=NnMxoCkO7q0xgL3wQ1vqr6OYT3jlwoce9oMbLA08nCIUVxFgb6x7VIT8la+xLmvQ26XFnb MF04jfCuET91GpvYPDj7H1tadTqU0js9tDqFqRWRNtejbqpfEiRKs4fQfRzhIEsIFnCd9z iX7n/o2p7BvCYuGYB3HrVyTTKJiXGJE= Received: from mail-lf1-f70.google.com (mail-lf1-f70.google.com [209.85.167.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-410-ow9qp0oeMbCref0LBNJJ6A-1; Fri, 08 Mar 2024 03:27:28 -0500 X-MC-Unique: ow9qp0oeMbCref0LBNJJ6A-1 Received: by mail-lf1-f70.google.com with SMTP id 2adb3069b0e04-5133f5243d7so655495e87.1 for ; Fri, 08 Mar 2024 00:27:27 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1709886446; x=1710491246; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=/tjSf3tXAxvCIt4Px1vaAqYpZ/2jCPC4l0Wy/FpoZBM=; b=No/dEAe+3uSMCh2rhqrqiSd9Olt/YJxc8MUeAYeZYEbWhLAZg7ikUgMRIgiS08veXp iBTXTCi1FeYucffs0HPs3OZBJ7S3w+nY+IZ5qXabstT2P9wEbOmFXjpDqmHe1JDFOMOJ a94AVwPlwCCZrdvycN5L5sSSBM8SNJ36RUGna0HWrc2jiaHTcI1dmyfYd197MQoczAYk 2e33JcBr6Ur4julKsasq8zvzmAxkQA7rJgZEqJ93sUBBLeE+gS0jskZixuBO++hCF1j6 cj6wxaM60TGJaA4gPBzjCkvh/FgtKE135CtVmr7xbR/NzaMH6Y+PP85CrtbVC8QRPfQY dsaA== X-Gm-Message-State: AOJu0YzVaMbfjJjgqTe0Rj4FbO8EWnHRu2rMOgbrr6AtsAneQLvolmhu Y9Smc57PJT17WECUGnNnSRWNXWrmrqfbTsD37OHLkCM+INLu+GPwNNUP/Qag0Zscld5qSdoZMDk uSx5CBCzSZyth2uoOLtceYUMQO9EuQL2+Me8XCSM618OuRz2qNLUA4jHOfsJR5HIIAtMcIAlgu1 BVebq6uZpsqwJkHDg= X-Received: by 2002:a05:6512:34ce:b0:513:28c4:b91c with SMTP id w14-20020a05651234ce00b0051328c4b91cmr2459757lfr.64.1709886446590; Fri, 08 Mar 2024 00:27:26 -0800 (PST) X-Google-Smtp-Source: AGHT+IH3R5IGSrXfd6bBm1tVch4XdCRvUGY5Ol1Ryftwq0u8DW/5tDzysHTHkpcGLXbroVbwYhx4QQqqeDZG318HqbY= X-Received: by 2002:a05:6512:34ce:b0:513:28c4:b91c with SMTP id w14-20020a05651234ce00b0051328c4b91cmr2459749lfr.64.1709886446205; Fri, 08 Mar 2024 00:27:26 -0800 (PST) MIME-Version: 1.0 References: <20230927150854.3670391-2-paul.szczepanek@arm.com> <20240307203943.188101-1-paul.szczepanek@arm.com> In-Reply-To: <20240307203943.188101-1-paul.szczepanek@arm.com> From: David Marchand Date: Fri, 8 Mar 2024 09:27:14 +0100 Message-ID: Subject: Re: [PATCH v7 0/4] add pointer compression API To: Paul Szczepanek Cc: dev@dpdk.org, Honnappa Nagarahalli , Thomas Monjalon , "Mcnamara, John" X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com 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 Hello Paul, On Thu, Mar 7, 2024 at 9:40=E2=80=AFPM Paul Szczepanek wrote: > > This patchset is proposing adding a new EAL header with utility functions > that allow compression of arrays of pointers. > > When passing caches full of pointers between threads, memory containing > the pointers is copied multiple times which is especially costly between > cores. A compression method will allow us to shrink the memory size > copied. > > The compression takes advantage of the fact that pointers are usually > located in a limited memory region (like a mempool). We can compress them > by converting them to offsets from a base memory address. > > Offsets can be stored in fewer bytes (dictated by the memory region size > and alignment of the pointer). For example: an 8 byte aligned pointer > which is part of a 32GB memory pool can be stored in 4 bytes. The API is > very generic and does not assume mempool pointers, any pointer can be > passed in. > > Compression is based on few and fast operations and especially with vecto= r > instructions leveraged creates minimal overhead. > > The API accepts and returns arrays because the overhead means it only is > worth it when done in bulk. > > Test is added that shows potential performance gain from compression. In > this test an array of pointers is passed through a ring between two cores= . > It shows the gain which is dependent on the bulk operation size. In this > synthetic test run on ampere altra a substantial (up to 25%) performance > gain is seen if done in bulk size larger than 32. At 32 it breaks even an= d > lower sizes create a small (less than 5%) slowdown due to overhead. > > In a more realistic mock application running the l3 forwarding dpdk > example that works in pipeline mode on two cores this translated into a > ~5% throughput increase on an ampere altra. > > v2: > * addressed review comments (style, explanations and typos) > * lowered bulk iterations closer to original numbers to keep runtime shor= t > * fixed pointer size warning on 32-bit arch > v3: > * added 16-bit versions of compression functions and tests > * added documentation of these new utility functions in the EAL guide > v4: > * added unit test > * fix bug in NEON implementation of 32-bit decompress > v5: > * disable NEON and SVE implementation on AARCH32 due to wrong pointer siz= e > v6: > * added example usage to commit message of the initial commit > v7: > * rebase to remove clashing mailmap changes > v8: > * put ptr compress into its own library > * add depends-on tag > * remove copyright bumps > * typos > > Paul Szczepanek (4): > ptr_compress: add pointer compression library > test: add pointer compress tests to ring perf test > docs: add pointer compression guide > test: add unit test for ptr compression > > app/test/meson.build | 21 +- > app/test/test_ptr_compress.c | 108 +++++++ > app/test/test_ring.h | 92 ++++++ > app/test/test_ring_perf.c | 352 ++++++++++++++------- > doc/guides/prog_guide/ptr_compress_lib.rst | 144 +++++++++ > lib/meson.build | 1 + > lib/ptr_compress/meson.build | 4 + > lib/ptr_compress/rte_ptr_compress.h | 266 ++++++++++++++++ > lib/ptr_compress/version.map | 3 + > 9 files changed, 859 insertions(+), 132 deletions(-) > create mode 100644 app/test/test_ptr_compress.c > create mode 100644 doc/guides/prog_guide/ptr_compress_lib.rst > create mode 100644 lib/ptr_compress/meson.build > create mode 100644 lib/ptr_compress/rte_ptr_compress.h > create mode 100644 lib/ptr_compress/version.map We mentionned during the weekly release meeting, it seemed too late for merging this work in the 24.03 release. Looking at v8, I have comments on this series: - rather than put a Depends-on: tag, take the lib: patch as part of your series, there is no need for this patch without the ptr_compress lib and it will avoid any CI issue (ovsrobot does not support Depends-on: patch- for example), - lib/ptr_compress/version.map is unneeded now, - lib/ptr_compress/, app/test/test_ptr_compress.c and doc/guides/prog_guide/ptr_compress_lib.rst need a MAINTAINERS entry, - prefer lowercase characters for mail addresses in commitlogs, - the documentation is not referenced in doc/guides/prog_guide/index.rst, - doxygen does not know of this new library, you must update doc/api/doxy-api-index.md and doc/api/doxy-api.conf.in, - a RN entry is missing, There were also comments on the lib: patch. At this point, it is better to take our time to finish putting this work in good form and merge it in 24.07. Thanks. --=20 David Marchand