[ovs-dev] [PATCH v3 03/12] flow: Skip invoking expensive count_1bits() with zero input.

[Fischetti, Antonio]

Thanks Jarno, one question below.

Antonio

> -----Original Message-----
> From: dev [mailto:dev-bounces at openvswitch.org] On Behalf Of Jarno
> Rajahalme
> Sent: Friday, October 14, 2016 5:03 PM
> To: Bodireddy, Bhanuprakash <bhanuprakash.bodireddy at intel.com>
> Cc: dev at 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
> <bhanuprakash.bodireddy at intel.com> 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 <bhanuprakash.bodireddy at intel.com>
> > Co-authored-by: Antonio Fischetti <antonio.fischetti at intel.com>
> > Signed-off-by: Antonio Fischetti <antonio.fischetti at intel.com>
> > Acked-by: Jarno Rajahalme <jarno at ovn.org>
> > ---
> > 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 at openvswitch.org
> > http://openvswitch.org/mailman/listinfo/dev
> 
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev