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 <step...@networkplumber.org> > Sent: Monday, April 1, 2024 9:53 PM > To: Honnappa Nagarahalli <honnappa.nagaraha...@arm.com> > Cc: Aditya Ambadipudi <aditya.ambadip...@arm.com>; dev@dpdk.org > <dev@dpdk.org>; jack...@nvidia.com <jack...@nvidia.com>; ma...@nvidia.com > <ma...@nvidia.com>; viachesl...@nvidia.com <viachesl...@nvidia.com>; > roret...@linux.microsoft.com <roret...@linux.microsoft.com>; > konstantin.v.anan...@yandex.ru <konstantin.v.anan...@yandex.ru>; > konstantin.anan...@huawei.com <konstantin.anan...@huawei.com>; > m...@smartsharesystems.com <m...@smartsharesystems.com>; > hof...@lysator.liu.se <hof...@lysator.liu.se>; Dhruv Tripathi > <dhruv.tripa...@arm.com>; Wathsala Wathawana Vithanage > <wathsala.vithan...@arm.com>; nd <n...@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.nagaraha...@arm.com> wrote: > > > > On Apr 1, 2024, at 9:00 PM, Stephen Hemminger > > > <step...@networkplumber.org> wrote: > > > > > > On Tue, 2 Apr 2024 01:35:28 +0000 > > > Honnappa Nagarahalli <honnappa.nagaraha...@arm.com> wrote: > > > > > >>> On Apr 1, 2024, at 7:47 PM, Stephen Hemminger > > >>> <step...@networkplumber.org> wrote: > > >>> > > >>> On Mon, 1 Apr 2024 22:28:52 +0000 > > >>> Aditya Ambadipudi <aditya.ambadip...@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
