DPDK patches and discussions
 help / color / mirror / Atom feed
From: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
To: David Marchand <david.marchand@redhat.com>
Cc: Paul Szczepanek <Paul.Szczepanek@arm.com>,
	"dev@dpdk.org" <dev@dpdk.org>,
	 "thomas@monjalon.net" <thomas@monjalon.net>,
	"Mcnamara, John" <john.mcnamara@intel.com>, nd <nd@arm.com>,
	Wathsala Wathawana Vithanage <wathsala.vithanage@arm.com>
Subject: Re: [PATCH v7 0/4] add pointer compression API
Date: Sun, 10 Mar 2024 19:34:48 +0000	[thread overview]
Message-ID: <E1F7C643-9529-41AA-9ACE-6116B35592A8@arm.com> (raw)
In-Reply-To: <CAJFAV8zpmdQ-NM5EX=6dHAWX00T__OYV_d2Ei8XwdVKoKab8Lw@mail.gmail.com>

+ Wathsala


> On Mar 8, 2024, at 2:27 AM, David Marchand <david.marchand@redhat.com> wrote:
> 
> Hello Paul,
> 
> On Thu, Mar 7, 2024 at 9:40 PM Paul Szczepanek <paul.szczepanek@arm.com> 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 vector
>> 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 and
>> 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 short
>> * 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 size
>> 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),
Agree, this is a better solution

> - 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,
Apologies for missing these.

> 
> There were also comments on the lib: patch.
Not sure which comments you are talking about. Your comments on V7 were addressed in V8.

> 
> At this point, it is better to take our time to finish putting this
> work in good form and merge it in 24.07.
Given your comments do not affect the code and the changes are pretty straightforward, request you reconsider the decision.
Anyway, we will get these changes pushed to community on Monday.

> 
> Thanks.
> 
> -- 
> David Marchand
> 


  reply	other threads:[~2024-03-10 19:35 UTC|newest]

Thread overview: 76+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-09-27 15:08 [RFC 0/2] " Paul Szczepanek
2023-09-27 15:08 ` [RFC 1/2] eal: add pointer compression functions Paul Szczepanek
2023-10-09 15:54   ` Thomas Monjalon
2023-10-11 13:36     ` Honnappa Nagarahalli
2023-10-11 16:43       ` Paul Szczepanek
2023-10-11 12:43   ` [RFC v2 0/2] add pointer compression API Paul Szczepanek
2023-10-11 12:43     ` [RFC v2 1/2] eal: add pointer compression functions Paul Szczepanek
2023-10-11 12:43     ` [RFC v2 2/2] test: add pointer compress tests to ring perf test Paul Szczepanek
2023-10-31 18:10   ` [PATCH v3 0/3] add pointer compression API Paul Szczepanek
2023-10-31 18:10     ` [PATCH v3 1/3] eal: add pointer compression functions Paul Szczepanek
2023-10-31 18:10     ` [PATCH v3 2/3] test: add pointer compress tests to ring perf test Paul Szczepanek
2023-10-31 18:10     ` [PATCH v3 3/3] docs: add pointer compression to the EAL guide Paul Szczepanek
2023-11-01  7:42     ` [PATCH v3 0/3] add pointer compression API Morten Brørup
2023-11-01 12:52       ` Paul Szczepanek
2023-11-01 12:46   ` [PATCH v4 0/4] " Paul Szczepanek
2023-11-01 12:46     ` [PATCH v4 1/4] eal: add pointer compression functions Paul Szczepanek
2023-11-01 12:46     ` [PATCH v4 2/4] test: add pointer compress tests to ring perf test Paul Szczepanek
2023-11-01 12:46     ` [PATCH v4 3/4] docs: add pointer compression to the EAL guide Paul Szczepanek
2023-11-01 12:46     ` [PATCH v4 4/4] test: add unit test for ptr compression Paul Szczepanek
2023-11-01 18:12   ` [PATCH v5 0/4] add pointer compression API Paul Szczepanek
2023-11-01 18:12     ` [PATCH v5 1/4] eal: add pointer compression functions Paul Szczepanek
2024-02-11 15:32       ` Konstantin Ananyev
2023-11-01 18:12     ` [PATCH v5 2/4] test: add pointer compress tests to ring perf test Paul Szczepanek
2023-11-01 18:13     ` [PATCH v5 3/4] docs: add pointer compression to the EAL guide Paul Szczepanek
2023-11-01 18:13     ` [PATCH v5 4/4] test: add unit test for ptr compression Paul Szczepanek
2024-02-22  8:15     ` [PATCH v5 0/4] add pointer compression API Paul Szczepanek
2024-02-22 16:16       ` Konstantin Ananyev
2024-03-01 11:16         ` Morten Brørup
2024-03-01 16:12           ` Patrick Robb
2024-03-01 19:57           ` Honnappa Nagarahalli
2024-03-02 10:33             ` Morten Brørup
2024-03-06 22:31               ` Paul Szczepanek
2024-03-07  2:13                 ` Honnappa Nagarahalli
2024-03-04 14:44             ` Konstantin Ananyev
2024-05-15 17:00               ` Paul Szczepanek
2024-05-15 22:34                 ` Morten Brørup
2024-05-16  8:25                   ` Paul Szczepanek
2024-05-16  8:40                   ` Konstantin Ananyev
2024-02-29 16:03   ` [PATCH v6 " Paul Szczepanek
2024-02-29 16:03     ` [PATCH v6 1/4] eal: add pointer compression functions Paul Szczepanek
2024-02-29 16:03     ` [PATCH v6 2/4] test: add pointer compress tests to ring perf test Paul Szczepanek
2024-02-29 16:03     ` [PATCH v6 3/4] docs: add pointer compression to the EAL guide Paul Szczepanek
2024-02-29 16:03     ` [PATCH v6 4/4] test: add unit test for ptr compression Paul Szczepanek
2024-03-01 10:21   ` [PATCH v7 0/4] add pointer compression API Paul Szczepanek
2024-03-01 10:21     ` [PATCH v7 1/4] eal: add pointer compression functions Paul Szczepanek
2024-03-07 11:22       ` David Marchand
2024-03-01 10:21     ` [PATCH v7 2/4] test: add pointer compress tests to ring perf test Paul Szczepanek
2024-03-07 11:27       ` David Marchand
2024-03-01 10:21     ` [PATCH v7 3/4] docs: add pointer compression to the EAL guide Paul Szczepanek
2024-03-01 10:21     ` [PATCH v7 4/4] test: add unit test for ptr compression Paul Szczepanek
2024-03-07 11:30       ` David Marchand
2024-03-07 20:39   ` [PATCH v7 0/4] add pointer compression API Paul Szczepanek
2024-03-07 20:39     ` [PATCH v8 1/4] ptr_compress: add pointer compression library Paul Szczepanek
2024-03-07 20:39     ` [PATCH v8 2/4] test: add pointer compress tests to ring perf test Paul Szczepanek
2024-03-07 20:39     ` [PATCH v8 3/4] docs: add pointer compression guide Paul Szczepanek
2024-03-07 20:39     ` [PATCH v8 4/4] test: add unit test for ptr compression Paul Szczepanek
2024-03-08  8:27     ` [PATCH v7 0/4] add pointer compression API David Marchand
2024-03-10 19:34       ` Honnappa Nagarahalli [this message]
2024-03-11  7:44         ` David Marchand
2024-03-11 14:47   ` [PATCH v9 0/5] " Paul Szczepanek
2024-03-11 14:47     ` [PATCH v9 1/5] lib: allow libraries with no sources Paul Szczepanek
2024-03-11 15:23       ` Bruce Richardson
2024-03-15  8:33         ` Paul Szczepanek
2024-03-11 14:47     ` [PATCH v9 2/5] ptr_compress: add pointer compression library Paul Szczepanek
2024-03-11 14:47     ` [PATCH v9 3/5] test: add pointer compress tests to ring perf test Paul Szczepanek
2024-03-11 14:47     ` [PATCH v9 4/5] docs: add pointer compression guide Paul Szczepanek
2024-03-11 14:47     ` [PATCH v9 5/5] test: add unit test for ptr compression Paul Szczepanek
2024-03-11 20:31   ` [PATCH v10 0/5] add pointer compression API Paul Szczepanek
2024-03-11 20:31     ` [PATCH v10 1/5] lib: allow libraries with no sources Paul Szczepanek
2024-03-15  9:14       ` Bruce Richardson
2024-03-11 20:31     ` [PATCH v10 2/5] ptr_compress: add pointer compression library Paul Szczepanek
2024-03-11 20:31     ` [PATCH v10 3/5] test: add pointer compress tests to ring perf test Paul Szczepanek
2024-03-11 20:31     ` [PATCH v10 4/5] docs: add pointer compression guide Paul Szczepanek
2024-03-11 20:31     ` [PATCH v10 5/5] test: add unit test for ptr compression Paul Szczepanek
2023-09-27 15:08 ` [RFC 2/2] test: add pointer compress tests to ring perf test Paul Szczepanek
2023-10-09 15:48   ` Thomas Monjalon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=E1F7C643-9529-41AA-9ACE-6116B35592A8@arm.com \
    --to=honnappa.nagarahalli@arm.com \
    --cc=Paul.Szczepanek@arm.com \
    --cc=david.marchand@redhat.com \
    --cc=dev@dpdk.org \
    --cc=john.mcnamara@intel.com \
    --cc=nd@arm.com \
    --cc=thomas@monjalon.net \
    --cc=wathsala.vithanage@arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).