Re: [ovs-dev] [PATCH 06/12] cmap: Remove prefetching in cmap_find_batch().

[Bodireddy, Bhanuprakash]

>-Original Message-
>From: Jarno Rajahalme [mailto:ja...@ovn.org]
>Sent: Friday, October 7, 2016 10:10 PM
>To: Bodireddy, Bhanuprakash <bhanuprakash.bodire...@intel.com>
>Cc: dev@openvswitch.org
>Subject: Re: [ovs-dev] [PATCH 06/12] cmap: Remove prefetching in
>cmap_find_batch().
>
>
>> On Oct 7, 2016, at 9:17 AM, Bhanuprakash Bodireddy
><bhanuprakash.bodire...@intel.com> wrote:
>>
>> prefetching the data in to the caches isn't improving the performance
>> in cmap_find_batch(). Moreover its found that there is slight
>> improvement in performance with out prefetching.
>>
>
>I recall doing some performance testing for this earlier, but have no records 
>of
>the system or other circumstances. Is it likely that this is at least somewhat
>system and test case dependent?
Agree with you,  we are testing this on Haswell CPUs. 

 Also the modified batch size may be a
>factor here. 
I verified this patch with batch size '16' and still could get better 
performance.
So the improvement may not be due to increasing the batch size to 32 as done in 
first patch of the series. 

Anyway, I don’t currently have any proof to the contrary, so I
>have no problem removing the prefetching.
>
>However, you should also update all the comments referring to prefetching.
Sure, I will do this in v2.

Regards,
Bhanu Prakash.

>
>  Jarno
>
>> This patch removes prefetching from cmap_find_batch().
>>
>> Signed-off-by: Bhanuprakash Bodireddy
>> <bhanuprakash.bodire...@intel.com>
>> Signed-off-by: Antonio Fischetti <antonio.fische...@intel.com>
>> ---
>> lib/cmap.c | 4 
>> 1 file changed, 4 deletions(-)
>>
>> diff --git a/lib/cmap.c b/lib/cmap.c
>> index 8c7312d..4c34bda 100644
>> --- a/lib/cmap.c
>> +++ b/lib/cmap.c
>> @@ -397,7 +397,6 @@ cmap_find_batch(const struct cmap *cmap,
>unsigned long map,
>> ULLONG_FOR_EACH_1(i, map) {
>> h1s[i] = rehash(impl, hashes[i]);
>> b1s[i] = >buckets[h1s[i] & impl->mask];
>> -OVS_PREFETCH(b1s[i]);
>> }
>> /* Lookups, Round 1. Only look up at the first bucket. */
>> ULLONG_FOR_EACH_1(i, map) {
>> @@ -413,13 +412,11 @@ cmap_find_batch(const struct cmap *cmap,
>unsigned long map,
>> if (!node) {
>> /* Not found (yet); Prefetch the 2nd bucket. */
>> b2s[i] = >buckets[other_hash(h1s[i]) & impl->mask];
>> -OVS_PREFETCH(b2s[i]);
>> c1s[i] = c1; /* We may need to check this after Round 2. */
>> continue;
>> }
>> /* Found. */
>> ULLONG_SET0(map, i); /* Ignore this on round 2. */
>> -OVS_PREFETCH(node);
>> nodes[i] = node;
>> }
>> /* Round 2. Look into the 2nd bucket, if needed. */ @@ -453,7
>> +450,6 @@ cmap_find_batch(const struct cmap *cmap, unsigned long map,
>> continue;
>> }
>> found:
>> -OVS_PREFETCH(node);
>> nodes[i] = node;
>> }
>> return result;
>> --
>> 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

Re: [ovs-dev] [PATCH 06/12] cmap: Remove prefetching in cmap_find_batch().

[Jarno Rajahalme]

> On Oct 7, 2016, at 9:17 AM, Bhanuprakash Bodireddy 
>  wrote:
> 
> prefetching the data in to the caches isn't improving the performance in
> cmap_find_batch(). Moreover its found that there is slight improvement
> in performance with out prefetching.
> 

I recall doing some performance testing for this earlier, but have no records 
of the system or other circumstances. Is it likely that this is at least 
somewhat system and test case dependent? Also the modified batch size may be a 
factor here. Anyway, I don’t currently have any proof to the contrary, so I 
have no problem removing the prefetching.

However, you should also update all the comments referring to prefetching.

  Jarno

> This patch removes prefetching from cmap_find_batch().
> 
> Signed-off-by: Bhanuprakash Bodireddy 
> Signed-off-by: Antonio Fischetti 
> ---
> lib/cmap.c | 4 
> 1 file changed, 4 deletions(-)
> 
> diff --git a/lib/cmap.c b/lib/cmap.c
> index 8c7312d..4c34bda 100644
> --- a/lib/cmap.c
> +++ b/lib/cmap.c
> @@ -397,7 +397,6 @@ cmap_find_batch(const struct cmap *cmap, unsigned long 
> map,
> ULLONG_FOR_EACH_1(i, map) {
> h1s[i] = rehash(impl, hashes[i]);
> b1s[i] = >buckets[h1s[i] & impl->mask];
> -OVS_PREFETCH(b1s[i]);
> }
> /* Lookups, Round 1. Only look up at the first bucket. */
> ULLONG_FOR_EACH_1(i, map) {
> @@ -413,13 +412,11 @@ cmap_find_batch(const struct cmap *cmap, unsigned long 
> map,
> if (!node) {
> /* Not found (yet); Prefetch the 2nd bucket. */
> b2s[i] = >buckets[other_hash(h1s[i]) & impl->mask];
> -OVS_PREFETCH(b2s[i]);
> c1s[i] = c1; /* We may need to check this after Round 2. */
> continue;
> }
> /* Found. */
> ULLONG_SET0(map, i); /* Ignore this on round 2. */
> -OVS_PREFETCH(node);
> nodes[i] = node;
> }
> /* Round 2. Look into the 2nd bucket, if needed. */
> @@ -453,7 +450,6 @@ cmap_find_batch(const struct cmap *cmap, unsigned long 
> map,
> continue;
> }
> found:
> -OVS_PREFETCH(node);
> nodes[i] = node;
> }
> return result;
> -- 
> 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

[ovs-dev] [PATCH 06/12] cmap: Remove prefetching in cmap_find_batch().

[Bhanuprakash Bodireddy]

prefetching the data in to the caches isn't improving the performance in
cmap_find_batch(). Moreover its found that there is slight improvement
in performance with out prefetching.

This patch removes prefetching from cmap_find_batch().

Signed-off-by: Bhanuprakash Bodireddy 
Signed-off-by: Antonio Fischetti 
---
 lib/cmap.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/lib/cmap.c b/lib/cmap.c
index 8c7312d..4c34bda 100644
--- a/lib/cmap.c
+++ b/lib/cmap.c
@@ -397,7 +397,6 @@ cmap_find_batch(const struct cmap *cmap, unsigned long map,
 ULLONG_FOR_EACH_1(i, map) {
 h1s[i] = rehash(impl, hashes[i]);
 b1s[i] = >buckets[h1s[i] & impl->mask];
-OVS_PREFETCH(b1s[i]);
 }
 /* Lookups, Round 1. Only look up at the first bucket. */
 ULLONG_FOR_EACH_1(i, map) {
@@ -413,13 +412,11 @@ cmap_find_batch(const struct cmap *cmap, unsigned long 
map,
 if (!node) {
 /* Not found (yet); Prefetch the 2nd bucket. */
 b2s[i] = >buckets[other_hash(h1s[i]) & impl->mask];
-OVS_PREFETCH(b2s[i]);
 c1s[i] = c1; /* We may need to check this after Round 2. */
 continue;
 }
 /* Found. */
 ULLONG_SET0(map, i); /* Ignore this on round 2. */
-OVS_PREFETCH(node);
 nodes[i] = node;
 }
 /* Round 2. Look into the 2nd bucket, if needed. */
@@ -453,7 +450,6 @@ cmap_find_batch(const struct cmap *cmap, unsigned long map,
 continue;
 }
 found:
-OVS_PREFETCH(node);
 nodes[i] = node;
 }
 return result;
-- 
2.4.11

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev