RE: Optimizations are not features

["Morten Brørup" <mb@smartsharesystems.com>]

> From: Andrew Rybchenko [mailto:andrew.rybchenko@oktetlabs.ru]
> Sent: Saturday, 4 June 2022 14.52
> 
> On 6/4/22 15:19, Morten Brørup wrote:
> >> From: Jerin Jacob [mailto:jerinjacobk@gmail.com]
> >> Sent: Saturday, 4 June 2022 13.10
> >>
> >> On Sat, Jun 4, 2022 at 3:30 PM Andrew Rybchenko
> >> <andrew.rybchenko@oktetlabs.ru> wrote:
> >>>
> >>> On 6/4/22 12:33, Jerin Jacob wrote:
> >>>> On Sat, Jun 4, 2022 at 2:39 PM Morten Brørup
> >> <mb@smartsharesystems.com> wrote:
> >>>>>
> >>>>> I would like the DPDK community to change its view on compile
> time
> >> options. Here is why:
> >>>>>
> >>>>>
> >>>>>
> >>>>> Application specific performance micro-optimizations like “fast
> >> mbuf free” and “mbuf direct re-arm” are being added to DPDK and
> >> presented as features.
> >>>>>
> >>>>>
> >>>>>
> >>>>> They are not features, but optimizations, and I don’t understand
> >> the need for them to be available at run-time!
> >>>>>
> >>>>>
> >>>>>
> >>>>> Instead of adding a bunch of exotic exceptions to the fast path
> of
> >> the PMDs, they should be compile time options. This will improve
> >> performance by avoiding branches in the fast path, both for the
> >> applications using them, and for generic applications (where the
> exotic
> >> code is omitted).
> >>>>
> >>>> Agree. I think, keeping the best of both worlds would be
> >>>>
> >>>> -Enable the feature/optimization as runtime
> >>>> -Have a compile-time option to disable the feature/optimization as
> >> an override.
> >>>
> >>> It is hard to find the right balance, but in general compile
> >>> time options are a nightmare for maintenance. Number of
> >>> required builds will grow as an exponent.
> >
> > Test combinations are exponential for N features, regardless if N are
> runtime or compile time options.
> 
> But since I'm talking about build checks I don't care about exponential
> grows in run time. Yes, testing should care, but it is a separate
> story.

Build checks is just one of many test methods. A very low cost and efficient test method, though.

I acknowledge that build checks will be more complicated with additional compile time options. And build checks are important for code quality, so we should find a solution for that challenge.

The primary scope of my suggestion is application specific performance optimization features only, so let's call them "exotic features". Also, they should be "opt in". Keep in mind: They are not really features, since they don't add anything new, they are only performance optimizations beneficial for specific application use cases.

With this in mind, we could continue ordinary testing with these options disabled. I.e. no additional testing.

Regardless how exotic the features may be, we certainly want some testing of them, so we could do some things to avoid exponential testing:

1. With each ordinary build check (with all options disabled), another build check could be run with a random set of options enabled. Or additional tests could be run on a weekly basis with random combinations of options.

2. Any patch related to one or more of these exotic features could include some magic keywords to indicate which combinations of options must be enabled during build checking and testing.

> 
> >
> >>> Of course, we can
> >>> limit number of checked combinations, but it will result in
> >>> flow of patches to fix build in other cases.
> >>
> >> The build breakage can be fixed if we use (2) vs (1)
> >>
> >> 1)
> >> #ifdef ...
> >> My feature
> >> #endif
> >>
> >> 2)
> >> static __rte_always_inline int
> >> rte_has_xyz_feature(void)
> >> {
> >> #ifdef RTE_LIBRTE_XYZ_FEATURE
> >>          return RTE_LIBRTE_XYZ_FEATURE;
> >> #else
> >>          return 0;
> >> #endif
> >> }
> >>
> >> if(rte_has_xyz_feature())) {
> >> My feature code
> >>
> >> }
> >>
> 
> Jerin, thanks, very good example.
> 
> > I'm not sure all the features can be covered by that, e.g. added
> fields in structures.
> 
> +1
> 
> >
> > Also, I would consider such features "opt in" at compile time only.
> As such, they could be allowed to break the ABI/API.
> >
> >>
> >>
> >>> Also compile time options tend to make code less readable
> >>> which makes all aspects of the development harder.
> >>>
> >>> Yes, compile time is nice for micro optimizations, but
> >>> I have great concerns that it is a right way to go.
> >>>
> >>>>> Please note that I am only talking about the performance
> >> optimizations that are limited to application specific use cases. I
> >> think it makes sense to require that performance optimizing an
> >> application also requires recompiling the performance critical
> >> libraries used by it.
> >>>>>
> >>>>>
> >>>>>
> >>>>> Allowing compile time options for application specific
> performance
> >> optimizations in DPDK would also open a path for other
> optimizations,
> >> which can only be achieved at compile time, such as “no fragmented
> >> packets”, “no attached mbufs” and “single mbuf pool”. And even more
> >> exotic optimizations, such as the “indexed mempool cache”, which was
> >> rejected due to ABI violations – they could be marked as “risky and
> >> untested” or similar, but still be part of the DPDK main repository.
> >>>>>
> >>>>>
> >>>>>
> >>>>>
> >>>>>
> >>>>> Med venlig hilsen / Kind regards,
> >>>>>
> >>>>> -Morten Brørup
> >>>>>
> >>>>>
> >>>
> >
>