Re: Optimizations are not features

[Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>]

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.

> 
>>> 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
>>>>>
>>>>>
>>>
>