[PATCH v1 0/2] deque: add multithread unsafe deque library

[Tyler Retzlaff]

On Tue, Apr 02, 2024 at 03:03:13AM +0000, Aditya Ambadipudi wrote:
> Hello Stephen,
> 
> I have a copy of CLRS with me. And Deque is a very standard word in computer science. Even CLRS which is considered one of the most foundational books in computer science uses the word deque.

i'm kind of inclined to agree with this. double ended queue is pretty
well known as ``deque`` perhaps most notably from stl.

https://en.cppreference.com/w/cpp/container/deque

i would however strongly advise that there be no use of ``deque`` bare
(without the rte_ prefix) in any public header. (i.e. inline function
variables , parameter names, etc...). that would almost certainly result
in frustration for C++ consumers of dpdk that may be doing the
following:

#include <deque>
#include <rte_deque.h>

using namespace std;

a quick pass of the patches and i don't see any instances without the
rte_ prefix so only cautioning that we would want to avoid it.

> 
> I don't think there is any better word to describe the datastructure we are building other than "deque".
> 
> Is there a way to add an exception for that word in the dictionary words check we run? I genuinely think the readability of this library that we are building will suffer if we don't use the word "deque" here.
> 
> Thank you,
> Aditya Ambadipudi
> 
> [image/jpeg][image/jpeg]
> 
> ________________________________
> From: Stephen Hemminger <stephen at networkplumber.org>
> Sent: Monday, April 1, 2024 9:53 PM
> To: Honnappa Nagarahalli <Honnappa.Nagarahalli at arm.com>
> Cc: Aditya Ambadipudi <Aditya.Ambadipudi at arm.com>; dev at dpdk.org <dev at dpdk.org>; jackmin at nvidia.com <jackmin at nvidia.com>; matan at nvidia.com <matan at nvidia.com>; viacheslavo at nvidia.com <viacheslavo at nvidia.com>; roretzla at linux.microsoft.com <roretzla at linux.microsoft.com>; konstantin.v.ananyev at yandex.ru <konstantin.v.ananyev at yandex.ru>; konstantin.ananyev at huawei.com <konstantin.ananyev at huawei.com>; mb at smartsharesystems.com <mb at smartsharesystems.com>; hofors at lysator.liu.se <hofors at lysator.liu.se>; Dhruv Tripathi <Dhruv.Tripathi at arm.com>; Wathsala Wathawana Vithanage <wathsala.vithanage at arm.com>; nd <nd at arm.com>
> Subject: Re: [PATCH v1 0/2] deque: add multithread unsafe deque library
> 
> On Tue, 2 Apr 2024 02:14:06 +0000
> Honnappa Nagarahalli <Honnappa.Nagarahalli at arm.com> wrote:
> 
> > > On Apr 1, 2024, at 9:00 PM, Stephen Hemminger <stephen at networkplumber.org> wrote:
> > >
> > > On Tue, 2 Apr 2024 01:35:28 +0000
> > > Honnappa Nagarahalli <Honnappa.Nagarahalli at arm.com> wrote:
> > >
> > >>> On Apr 1, 2024, at 7:47 PM, Stephen Hemminger <stephen at networkplumber.org> wrote:
> > >>>
> > >>> On Mon, 1 Apr 2024 22:28:52 +0000
> > >>> Aditya Ambadipudi <Aditya.Ambadipudi at arm.com> wrote:
> > >>>
> > >>>> Thanks, Stephen, for the comment.
> > >>>>
> > >>>> Unfortunately, we don't have the dev setup nor the resources to test out this change using MSVC.
> > >>>>
> > >>>> Thank you,
> > >>>> Aditya Ambadipudi
> > >>>
> > >>> All it requires is the community version of MSVC which is free. And setting up a Windows
> > >>> VM with KVM is free and easy.
> > >>>
> > >>> IMHO all new libraries have to build on all environments, unless they are enabling
> > >>> platform specific features.
> > >> I see that UNH CI is running Windows VMs, the tests are passing there. So, we do not need to anything.
> > >>
> > >
> > > That only tests the clang part.
> > > You need to modify lib/meson.build to get it tested with the windows compiler.
> > Any idea on when is this getting added to CI?
> 
> You need to add this to next version of the patch.
> I tried it and MSVC has no problems with the new code.
> 
> Another issue is the naming. Right now the choice of 'deque' generates lots of
> checkpatch errors, and every bit of new code that uses the library will get a warning
> as well. Can you think of a better name?
> 
> diff --git a/lib/meson.build b/lib/meson.build
> index 8c8c1e98e2..127e4dc68c 100644
> --- a/lib/meson.build
> +++ b/lib/meson.build
> @@ -75,6 +75,7 @@ if is_ms_compiler
>              'kvargs',
>              'telemetry',
>              'eal',
> +            'deque',
>              'ring',
>      ]
>  endif