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 < [email protected]> wrote: > On 22 June 2017 at 10:30, Maxim Uvarov <[email protected]> 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:[email protected]] > >>> Sent: Thursday, June 22, 2017 11:22 AM > >>> To: Honnappa Nagarahalli <[email protected]> > >>> Cc: Savolainen, Petri (Nokia - FI/Espoo) <[email protected]>; > lng-odp-forward <[email protected]> > >>> 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: > [email protected]> 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:[email protected]> 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:[email protected]] On > Behalf Of > >>>>> Honnappa Nagarahalli > >>>>> Sent: Tuesday, June 20, 2017 7:07 AM > >>>>> To: Bill Fischofer <mailto:[email protected]> > >>>>> Cc: lng-odp-forward <mailto:[email protected]> > >>>>> 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: > [email protected]> > >>>>> wrote: > >>>>>> On Mon, Jun 19, 2017 at 4:19 PM, Honnappa Nagarahalli > >>>>>> <mailto:[email protected]> wrote: > >>>>>>> Hi Bill/Maxim, > >>>>>>> I do not see any further comments, can we merge this to > api-next? > >>>>>>> > >>>>>>> Thanks, > >>>>>>> Honnappa > >>>> > >>>> > >>> > > >
