Re: [ovs-dev] [PATCH v3 03/12] flow: Skip invoking expensive count_1bits() with zero input.
> On Oct 17, 2016, at 2:10 AM, Fischetti, Antonio
> wrote:
>
> Thanks Jarno, one question below.
>
> Antonio
>
>> -Original Message-
>> From: dev [mailto:dev-boun...@openvswitch.org
>> <mailto:dev-boun...@openvswitch.org>] On Behalf Of Jarno
>> Rajahalme
>> Sent: Friday, October 14, 2016 5:03 PM
>> To: Bodireddy, Bhanuprakash > <mailto:bhanuprakash.bodire...@intel.com>>
>> Cc: dev@openvswitch.org <mailto:dev@openvswitch.org>
>> Subject: Re: [ovs-dev] [PATCH v3 03/12] flow: Skip invoking expensive
>> count_1bits() with zero input.
>>
>>
>>> On Oct 14, 2016, at 7:37 AM, Bhanuprakash Bodireddy
>> wrote:
>>>
>>> This patch checks if trash is non-zero and only then resets the flowmap
>>> bit and increment the pointer by set bits as found in trash.
>>>
>>> Signed-off-by: Bhanuprakash Bodireddy
>>> Co-authored-by: Antonio Fischetti
>>> Signed-off-by: Antonio Fischetti
>>> Acked-by: Jarno Rajahalme
>>> ---
>>> lib/flow.h | 15 ++-
>>> 1 file changed, 10 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/lib/flow.h b/lib/flow.h
>>> index 5a14941..74e75d6 100644
>>> --- a/lib/flow.h
>>> +++ b/lib/flow.h
>>> @@ -614,11 +614,16 @@ mf_get_next_in_map(struct mf_for_each_in_map_aux
>> *aux,
>>> * to ‘rm1bit’. */
>>>map_t trash = *fmap & (rm1bit - 1);
>>>
>>> -*fmap -= trash;
>>> -/* count_1bits() is fast for systems where speed matters (e.g.,
>>> - * DPDK), so we don't try avoid using it.
>>> - * Advance 'aux->values' to point to the value for 'rm1bit'. */
>>> -aux->values += count_1bits(trash);
>>> +/* Avoid resetting 'fmap' and calling count_1bits() when trash
>> is
>>> + * zero. */
>>> +if (trash) {
>>> +*fmap -= trash;
>>> +/* count_1bits() is fast for systems where speed matters
>> (e.g.,
>>> + * DPDK), so we don't try avoid using it.
>>
>> The comment above is still wrong as we now test ‘trash’ for non-zero
>> above.
>>
>> Jarno
> [Antonio F] Just to avoid misunderstanding, do you mean we can now entirely
> remove
> the existing comment
>/* count_1bits() is fast for systems where speed
> right?
> Because with the latest changes now we do try to avoid using count_1bits().
> Is that ok?
>
>
Yes.
Jarno
>>
>>> + * Advance 'aux->values' to point to the value for 'rm1bit'
>> only.
>>> + */
>>> +aux->values += count_1bits(trash);
>>> +}
>>>
>>>*value = *aux->values;
>>>} else {
>>> --
>>> 2.4.11
>>>
>>> ___
>>> dev mailing list
>>> dev@openvswitch.org
>>> http://openvswitch.org/mailman/listinfo/dev
>>
>> ___
>> dev mailing list
>> dev@openvswitch.org <mailto:dev@openvswitch.org>
>> http://openvswitch.org/mailman/listinfo/dev
>> <http://openvswitch.org/mailman/listinfo/dev>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v3 03/12] flow: Skip invoking expensive count_1bits() with zero input.
Thanks Jarno, one question below.
Antonio
> -Original Message-
> From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Jarno
> Rajahalme
> Sent: Friday, October 14, 2016 5:03 PM
> To: Bodireddy, Bhanuprakash
> Cc: dev@openvswitch.org
> Subject: Re: [ovs-dev] [PATCH v3 03/12] flow: Skip invoking expensive
> count_1bits() with zero input.
>
>
> > On Oct 14, 2016, at 7:37 AM, Bhanuprakash Bodireddy
> wrote:
> >
> > This patch checks if trash is non-zero and only then resets the flowmap
> > bit and increment the pointer by set bits as found in trash.
> >
> > Signed-off-by: Bhanuprakash Bodireddy
> > Co-authored-by: Antonio Fischetti
> > Signed-off-by: Antonio Fischetti
> > Acked-by: Jarno Rajahalme
> > ---
> > lib/flow.h | 15 ++-
> > 1 file changed, 10 insertions(+), 5 deletions(-)
> >
> > diff --git a/lib/flow.h b/lib/flow.h
> > index 5a14941..74e75d6 100644
> > --- a/lib/flow.h
> > +++ b/lib/flow.h
> > @@ -614,11 +614,16 @@ mf_get_next_in_map(struct mf_for_each_in_map_aux
> *aux,
> > * to ‘rm1bit’. */
> > map_t trash = *fmap & (rm1bit - 1);
> >
> > -*fmap -= trash;
> > -/* count_1bits() is fast for systems where speed matters (e.g.,
> > - * DPDK), so we don't try avoid using it.
> > - * Advance 'aux->values' to point to the value for 'rm1bit'. */
> > -aux->values += count_1bits(trash);
> > +/* Avoid resetting 'fmap' and calling count_1bits() when trash
> is
> > + * zero. */
> > +if (trash) {
> > +*fmap -= trash;
> > +/* count_1bits() is fast for systems where speed matters
> (e.g.,
> > + * DPDK), so we don't try avoid using it.
>
> The comment above is still wrong as we now test ‘trash’ for non-zero
> above.
>
> Jarno
[Antonio F] Just to avoid misunderstanding, do you mean we can now entirely
remove
the existing comment
/* count_1bits() is fast for systems where speed
right?
Because with the latest changes now we do try to avoid using count_1bits(). Is
that ok?
>
> > + * Advance 'aux->values' to point to the value for 'rm1bit'
> only.
> > + */
> > +aux->values += count_1bits(trash);
> > +}
> >
> > *value = *aux->values;
> > } else {
> > --
> > 2.4.11
> >
> > ___
> > dev mailing list
> > dev@openvswitch.org
> > http://openvswitch.org/mailman/listinfo/dev
>
> ___
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v3 03/12] flow: Skip invoking expensive count_1bits() with zero input.
> On Oct 14, 2016, at 7:37 AM, Bhanuprakash Bodireddy
> wrote:
>
> This patch checks if trash is non-zero and only then resets the flowmap
> bit and increment the pointer by set bits as found in trash.
>
> Signed-off-by: Bhanuprakash Bodireddy
> Co-authored-by: Antonio Fischetti
> Signed-off-by: Antonio Fischetti
> Acked-by: Jarno Rajahalme
> ---
> lib/flow.h | 15 ++-
> 1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/lib/flow.h b/lib/flow.h
> index 5a14941..74e75d6 100644
> --- a/lib/flow.h
> +++ b/lib/flow.h
> @@ -614,11 +614,16 @@ mf_get_next_in_map(struct mf_for_each_in_map_aux *aux,
> * to ‘rm1bit’. */
> map_t trash = *fmap & (rm1bit - 1);
>
> -*fmap -= trash;
> -/* count_1bits() is fast for systems where speed matters (e.g.,
> - * DPDK), so we don't try avoid using it.
> - * Advance 'aux->values' to point to the value for 'rm1bit'. */
> -aux->values += count_1bits(trash);
> +/* Avoid resetting 'fmap' and calling count_1bits() when trash is
> + * zero. */
> +if (trash) {
> +*fmap -= trash;
> +/* count_1bits() is fast for systems where speed matters (e.g.,
> + * DPDK), so we don't try avoid using it.
The comment above is still wrong as we now test ‘trash’ for non-zero above.
Jarno
> + * Advance 'aux->values' to point to the value for 'rm1bit' only.
> + */
> +aux->values += count_1bits(trash);
> +}
>
> *value = *aux->values;
> } else {
> --
> 2.4.11
>
> ___
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
