| Message ID | 1475857062-55311-5-git-send-email-bhanuprakash.bodireddy@intel.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Daniele Di Proietto |
| Headers | show |
> On Oct 7, 2016, at 9:17 AM, Bhanuprakash Bodireddy <bhanuprakash.bodireddy@intel.com> wrote: > > mhash_add__() is expensive and should be only called with valid input. > This patch will validate the input before invoking the mhash_add__ and > there by saving some cpu cycles. > > Signed-off-by: Bhanuprakash Bodireddy <bhanuprakash.bodireddy@intel.com> > Signed-off-by: Antonio Fischetti <antonio.fischetti@intel.com> > --- > lib/hash.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/lib/hash.h b/lib/hash.h > index 114a419..9bfebdb 100644 > --- a/lib/hash.h > +++ b/lib/hash.h > @@ -70,7 +70,7 @@ static inline uint32_t mhash_add__(uint32_t hash, uint32_t data) > > static inline uint32_t mhash_add(uint32_t hash, uint32_t data) > { > - hash = mhash_add__(hash, data); > + hash = data ? mhash_add__(hash, data): hash; IMO the zero check is best placed in the function mhash_add__(), where it is also more evident that zero-valued data would not change the hash anyway. Maybe a comment to that effect would be also nice? > hash = hash_rot(hash, 13); > return hash * 5 + 0xe6546b64; > } > -- > 2.4.11 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev
>-----Original Message----- >From: Jarno Rajahalme [mailto:jarno@ovn.org] >Sent: Friday, October 7, 2016 10:09 PM >To: Bodireddy, Bhanuprakash <bhanuprakash.bodireddy@intel.com> >Cc: dev@openvswitch.org >Subject: Re: [ovs-dev] [PATCH 04/12] hash: Skip invoking mhash_add__() with >zero input. > > >> On Oct 7, 2016, at 9:17 AM, Bhanuprakash Bodireddy ><bhanuprakash.bodireddy@intel.com> wrote: >> >> mhash_add__() is expensive and should be only called with valid input. >> This patch will validate the input before invoking the mhash_add__ and >> there by saving some cpu cycles. >> >> Signed-off-by: Bhanuprakash Bodireddy >> <bhanuprakash.bodireddy@intel.com> >> Signed-off-by: Antonio Fischetti <antonio.fischetti@intel.com> >> --- >> lib/hash.h | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/lib/hash.h b/lib/hash.h >> index 114a419..9bfebdb 100644 >> --- a/lib/hash.h >> +++ b/lib/hash.h >> @@ -70,7 +70,7 @@ static inline uint32_t mhash_add__(uint32_t hash, >> uint32_t data) >> >> static inline uint32_t mhash_add(uint32_t hash, uint32_t data) { >> - hash = mhash_add__(hash, data); >> + hash = data ? mhash_add__(hash, data): hash; > >IMO the zero check is best placed in the function mhash_add__(), where it is >also more evident that zero-valued data would not change the hash anyway. >Maybe a comment to that effect would be also nice? Agree, will do this in v2. Bhanu Prakash. > >> hash = hash_rot(hash, 13); >> return hash * 5 + 0xe6546b64; >> } >> -- >> 2.4.11 >> >> _______________________________________________ >> dev mailing list >> dev@openvswitch.org >> http://openvswitch.org/mailman/listinfo/dev
diff --git a/lib/hash.h b/lib/hash.h index 114a419..9bfebdb 100644 --- a/lib/hash.h +++ b/lib/hash.h @@ -70,7 +70,7 @@ static inline uint32_t mhash_add__(uint32_t hash, uint32_t data) static inline uint32_t mhash_add(uint32_t hash, uint32_t data) { - hash = mhash_add__(hash, data); + hash = data ? mhash_add__(hash, data): hash; hash = hash_rot(hash, 13); return hash * 5 + 0xe6546b64; }