On 06/26/17 13:50, Savolainen, Petri (Nokia - FI/Espoo) wrote:
> When init params are set to defaults...
> 
>       odp_init_param_init(&init);
> 
>       odp_init_global(&instance, &init, NULL);
> 
> 
> ... this patch causes -15% (about 1Mpps per cpu) packet rate degradation with 
> l2fwd. The timer poll function shows up as the second highest CPU cycle 
> consumer on perf. The defaults needs to be less aggressive: a tradeoff 
> between somewhat improved timer accuracy for some timer using applications 
> vs. performance degradation for all application using the scheduler (also 
> with the default inits)...
> 
> The patch actually disables the polling when passing NULL as init param. API 
> defines that NULL and odp_init_param_init(&init) result the same config ("all 
> defaults"). This implementation mismatch helps now when most applications 
> pass NULL, and thus timer polling is kept disabled by default.
> 
> When contributing to a central piece of data plane code, it would be really 
> important to measure packet rate before and after the change...
> 
> 
> -Petri
> 

Thank Petri,

I opened bug to track this:
https://bugs.linaro.org/show_bug.cgi?id=3078

Maxim.


>  
> 
> 
> 
> From: Maxim Uvarov [mailto:maxim.uva...@linaro.org] 
> Sent: Friday, June 23, 2017 5:23 PM
> To: Honnappa Nagarahalli <honnappa.nagaraha...@linaro.org>
> Cc: Brian Brooks <brian.bro...@arm.com>; Savolainen, Petri (Nokia - FI/Espoo) 
> <petri.savolai...@nokia.com>; lng-odp-forward <lng-odp@lists.linaro.org>
> Subject: Re: [lng-odp] [API-NEXT PATCH v4] timer: allow timer processing to 
> run on worker cores
> 
> Merged to api-next.
> I think this patch is clean and we can do further improvements / tuning later.
> Regards,
> Maxim.
> 
> On 22 June 2017 at 21:48, Honnappa Nagarahalli 
> <mailto:honnappa.nagaraha...@linaro.org> wrote:
> On 22 June 2017 at 10:30, Maxim Uvarov <mailto:maxim.uva...@linaro.org> wrote:
>> On 06/22/17 17:55, Brian Brooks wrote:
>>> On 06/22 10:27:01, Savolainen, Petri (Nokia - FI/Espoo) wrote:
>>>> I was asking to make sure that performance impact has been checked also 
>>>> when timers are not used, e.g. l2fwd performance before and after the 
>>>> change. It would be also appropriate to test impact in the worst case: 
>>>> l2fwd type application + a periodic 1sec timeout. Timer is on, but 
>>>> timeouts come very unfrequently (compared to packets).
>>>>
>>>> It seems that no performance tests were run, although the change affects 
>>>> performance of many applications (e.g. OFP has high packet rate with 
>>>> timers). Configuration options should be set with  defaults that are 
>>>> acceptable trade-off between packet processing performance and timeout 
>>>> accuracy.
>>>
>>> If timers are not used, the overhead is just checking a RO variable
>>> (post global init). If timers are used, CONFIG_ parameters have been
>>> provided. The defaults for these parameters came from the work to
>>> drastically reduce jitter of timer processing which is documented
>>> here [1] and presented at Linaro Connect here [2].
>>>
>>> If you speculate that these defaults might need to be changed, e.g.
>>> l2fwd, we welcome collaboration and data. But, this is not a blocking
>>> issue for this patch right now.
>>>
>>> [1] 
>>> https://docs.google.com/document/d/1sY7rOxqCNu-bMqjBiT5_keAIohrX1ZW-eL0oGLAQ4OM/edit?usp=sharing
>>> [2] http://connect.linaro.org/resource/bud17/bud17-320/
>>>
>>
>> 1) we have all adjustable configs here
>> ./platform/linux-generic/include/odp_config_internal.h
>> that might be also needs to be there.
>>
> That file has all the global config values. These are internal to this
> timer implementation, hence they do not need to be moved.
>>
>> 2) Do we need something special in CI to check different config values?
> 
> Nope.
>>
>> 3) Why it's compile time config values and not run time?
> 
> These config values are particular to this timer implementation.
> Similar to config values in
> ./platform/linux-generic/include/odp_config_internal.h, these also
> will be compile time constants.
> 
>>
>> Maxim.
>>
>>
>>>> -Petri
>>>>
>>>>
>>>> From: Maxim Uvarov [mailto:mailto:maxim.uva...@linaro.org]
>>>> Sent: Thursday, June 22, 2017 11:22 AM
>>>> To: Honnappa Nagarahalli <mailto:honnappa.nagaraha...@linaro.org>
>>>> Cc: Savolainen, Petri (Nokia - FI/Espoo) 
>>>> <mailto:petri.savolai...@nokia.com>; lng-odp-forward 
>>>> <mailto:lng-odp@lists.linaro.org>
>>>> Subject: Re: [lng-odp] [API-NEXT PATCH v4] timer: allow timer processing 
>>>> to run on worker cores
>>>>
>>>> Petri, do you want to test performance before patch inclusion?
>>>> Maxim.
>>>>
>>>> On 21 June 2017 at 21:52, Honnappa Nagarahalli 
>>>> <mailto:mailto:honnappa.nagaraha...@linaro.org> wrote:
>>>> We have not run any performance application. In our Linaro connect
>>>> meeting, we presented numbers on how it improves the timer resolution.
>>>> At this point, there is enough configuration options to control the
>>>> effect of calling timer in the scheduler. For applications that do not
>>>> want to use the timer, there should not be any change. For
>>>> applications that use timers non-frequently, the check frequency can
>>>> be controlled via the provided configuration options.
>>>>
>>>> On 20 June 2017 at 02:34, Savolainen, Petri (Nokia - FI/Espoo)
>>>> <mailto:mailto:petri.savolai...@nokia.com> wrote:
>>>>> Do you have some performance numbers? E.g. how much this slows down an 
>>>>> application which does not use timers (e.g. l2fwd), or an application 
>>>>> that uses only few, non-frequent timeouts?
>>>>>
>>>>> Additionally, init.h/feature.h is not yet in api-next - so this would not 
>>>>> build yet.
>>>>>
>>>>>
>>>>> -Petri
>>>>>
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: lng-odp 
>>>>>> [mailto:mailto:mailto:mailto:lng-odp-boun...@lists.linaro.org] On Behalf 
>>>>>> Of
>>>>>> Honnappa Nagarahalli
>>>>>> Sent: Tuesday, June 20, 2017 7:07 AM
>>>>>> To: Bill Fischofer <mailto:mailto:bill.fischo...@linaro.org>
>>>>>> Cc: lng-odp-forward <mailto:mailto:lng-odp@lists.linaro.org>
>>>>>> Subject: Re: [lng-odp] [API-NEXT PATCH v4] timer: allow timer processing
>>>>>> to run on worker cores
>>>>>>
>>>>>> Are you saying we should be good to merge this now?
>>>>>>
>>>>>> On 19 June 2017 at 17:42, Bill Fischofer 
>>>>>> <mailto:mailto:bill.fischo...@linaro.org>
>>>>>> wrote:
>>>>>>> On Mon, Jun 19, 2017 at 4:19 PM, Honnappa Nagarahalli
>>>>>>> <mailto:mailto:honnappa.nagaraha...@linaro.org> wrote:
>>>>>>>> Hi Bill/Maxim,
>>>>>>>>       I do not see any further comments, can we merge this to api-next?
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Honnappa
>>>>>
>>>>>
>>>>
>>
> 

Reply via email to