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:[email protected] >> <mailto:[email protected]>] On Behalf Of Jarno >> Rajahalme >> Sent: Friday, October 14, 2016 5:03 PM >> To: Bodireddy, Bhanuprakash > <mailto:[email protected]>> >> Cc: [email protected] <mailto:[email protected]> >> 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 >>> [email protected] >>> http://openvswitch.org/mailman/listinfo/dev >> >> ___ >> dev mailing list >> [email protected] <mailto:[email protected]> >> http://openvswitch.org/mailman/listinfo/dev >> <http://openvswitch.org/mailman/listinfo/dev> ___ dev mailing list [email protected] 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:[email protected]] On Behalf Of Jarno > Rajahalme > Sent: Friday, October 14, 2016 5:03 PM > To: Bodireddy, Bhanuprakash > Cc: [email protected] > 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 > > [email protected] > > http://openvswitch.org/mailman/listinfo/dev > > ___ > dev mailing list > [email protected] > http://openvswitch.org/mailman/listinfo/dev ___ dev mailing list [email protected] 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
> [email protected]
> http://openvswitch.org/mailman/listinfo/dev
___
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
