On 22 June 2017 at 02:48, Savolainen, Petri (Nokia - FI/Espoo) <petri.savolai...@nokia.com> wrote: > > >> -----Original Message----- >> From: lng-odp [mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of >> Honnappa Nagarahalli >> Sent: Thursday, June 22, 2017 12:20 AM >> To: Maxim Uvarov <maxim.uva...@linaro.org> >> Cc: lng-odp-forward <lng-odp@lists.linaro.org> >> Subject: Re: [lng-odp] [PATCH API-NEXT v1 1/1] linux-generic: queue: >> modular queue interface >> >> Agree, will rebase and update once we have the scalable queues merged. >> >> On 21 June 2017 at 14:53, Maxim Uvarov <maxim.uva...@linaro.org> wrote: >> > On 06/19/17 08:00, Github ODP bot wrote: >> >> log | 1269 >> ++++++++++++++++++++ >> > >> > log has not be a part of patch. >> > >> > Maxim. > > To be sure, I repeat my opinion about this RFC again. The internal queue > pointer (or table index - format does not matter) is needed to avoid doing > the same (API handle <-> qentry) conversion multiple times. We are inside > implementation, and it improves performance when the number of conversions > per packet is minimized. That's why there were direct qentry access outside > of queue.c in the first place.
I would think the direct qentry access outside queue.c was a bad choice from modularity perspective. It had resulted in spaghetti code for a benefit which is extremely small or non existence. The fear about the performance degradation needs to be measured and proved. The intention here is to prove that, this change does not affect performance with additions of few math operations. > > I'll look into the internal queue interface definition to optimize it. This > current version is just a dummy, copy-paste replacement of qentry->xxx with > fn_queue->xxx(). Which is OK for the first stage, but certainly not > optimal/final solution. If the opinion is it is dummy and a copy-paste I would say you have not understood the thought process behind this change. This change removes the need for having an internal abstract type while keeping the external handle as a 32b value (the fact that the 32b value is being stored in a 64b handle is a different matter). This is a big deal in keeping the code simple and understandable, especially when there is no impact on performance. This interface will be used as an example for other modules, which means there will not be any additional internal opaque data types as well. To repeat, I will rebase this once scalable scheduler is merged. > > -Petri >
