Re: [dpdk-dev] [RFC] ring: make ring implementation non-inlined

[Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>]

<snip>
> > >
> > > > Subject: Re: [dpdk-dev] [RFC] ring: make ring implementation non-
> > > inlined
> > > >
> > > > 26/03/2020 09:04, Morten Brørup:
> > > > > From: Jerin Jacob
> > > > > > On Fri, Mar 20, 2020 Konstantin Ananyev wrote:
> > > > > > >
> > > > > > > As was discussed here:
> > > > > > > http://mails.dpdk.org/archives/dev/2020-February/158586.html
> > > > > > > this RFC aimed to hide ring internals into .c and make all
> > > > > > > ring functions non-inlined. In theory that might help to
> > > > > > > maintain
> > > ABI
> > > > > > > stability in future.
> > > > > > > This is just a POC to measure the impact of proposed idea,
> > > proper
> > > > > > > implementation would definetly need some extra effort.
> > > > > > > On IA box (SKX) ring_perf_autotest shows ~20-30 cycles extra
> > > for
> > > > > > > enqueue+dequeue pair. On some more realistic code, I suspect
> > > > > > > the impact it might be a bit higher.
> > > > > > > For MP/MC bulk transfers degradation seems quite small,
> > > > > > > though
> > > for
> > > > > > > SP/SC and/or small transfers it is more then noticable (see
> > > exact
> > > > > > > numbers below).
> > > > > > > From my perspective we'd probably keep it inlined for now to
> > > avoid
> > > > > > > any non-anticipated perfomance degradations.
> > > > > > > Though intersted to see perf results and opinions from other
> > > > > > > interested parties.
> > > > > >
> > > > > > +1
> > > >
> > > > Konstantin, thank you for doing some measures
> > > >
> > > >
> > > > > > My reasoning is a bit different, DPDK is using in embedded
> > > > > > boxes
> > > too
> > > > > > where performance has more weight than ABI stuff.
> > > > >
> > > > > As a network appliance vendor I can confirm that we certainly
> > > > > care more about performance than ABI stability.
> > > > > ABI stability is irrelevant for us; and API instability is a
> > > > > non-recurring engineering cost each time
> > > we
> > > > > choose to switch to a new DPDK version, which we only do if we
> > > cannot
> > > > > avoid it, e.g. due to new drivers, security fixes or new
> > > > > features
> > > that
> > > > > we want to use.
> > > > >
> > > > > For us, the trend pointed in the wrong direction when DPDK
> > > > > switched the preference towards runtime configurability and
> > > > > deprecated
> > > compile
> > > > > time configurability. I do understand the reasoning behind it,
> > > > > and
> > > the
> > > > > impact is minimal, so we accept it.
> > > >
> > > > The code can be optimized by removing some instructions with #ifdef.
> > > > But the complexity of managing #ifdef enabling/disabling,
> > > > depending
> > > on the
> > > > platform and the use case, would be huge.
> > > > We try to have a reasonable code "always enabled" which performs
> > > > well
> > > in all
> > > > cases. This is a design choice which makes DPDK a library, not a
> > > > pool
> > > of code
> > > > to cherry-pick.
> > > >
> > > > > However, if DPDK starts sacrificing performance of the core
> > > libraries
> > > > > for the benefits of the GNU/Linux distributors, network
> > > > > appliance vendors may put more effort into sticking with old
> > > > > DPDK versions instead of updating.
> > > >
> > > > The initial choice regarding ABI compatibility was "do not care".
> > > > Recently, the decision was done to care about ABI compatibility as
> > > priority
> > > > number 2. The priority number 1 remains the performance.
> > > > That's a reason for allowing some ABI breakages in some specific
> > > releases
> > > > announced in advance.
> > > >
> > > > > > I think we need to focus first on slow path APIs ABI stuff.
> > > >
> > > > Yes we should not degrade fast path performance for the sake of
> > > avoiding
> > > > uncertain future ABI issues.
> > > >
> > > > Morten, Jerin, thank you for the feedback.
> > > I think we have a consensus here not to make any changes to inline
> > > functions for now.
> > > Should we mark this as 'Deferred or Rejected'?
> >
> > Rejected.
> >
> > There is no need for this modification now, and no actual use cases
> > for it in the road map. In other words: This modification has no use cases; it
> is purely academic. Many other suggestions have been rejected for the reason
> that they have no current use cases.
> >
> > As Thomas mentioned, DPDK has transitioned towards being a library,
> > rather than a pool of code to cherry-pick from. I have learned to live with
> this.
> >
> > Being a library doesn't mean that functions cannot be exposed as
> > inline code in the library header files. DPDK is mainly a high
> > performance library with a tradition of exposing many of its internals in its
> API, and we should keep it this way. We certainly don't want an opaque API
> hiding all of its internals, passing around void pointers.
> >
> > However, it was still an interesting experiment to investigate the
> performance cost.
> 
> Yes, please reject it.
I just tried to mark it rejected in patchwork, I do not have the permissions (probably you are the owner of the patch). Can you please mark it?

> Konstantin
>