[ovs-dev] [PATCH] cmap: Fix cmap_next_position()

Daniele Di Proietto ddiproietto at vmware.com
Wed Jul 16 17:52:44 UTC 2014


I’m running from the test suite, with GCC 4.9

It was slow because on my dev machine I compile without optimization (my bad). With optimizations:

- without testing cmap_next_position() : 8.6s
- if we always test also cmap_next_position() always (like you suggested): 18.8s

So I’m posting v2 with your suggestion applied

Thanks,

Daniele

On Jul 16, 2014, at 10:33 AM, Ben Pfaff <blp at nicira.com> wrote:

> There's a couple of reasons that test-cmap can be slow.
> 
> First, if you run it without a numerical argument, it iterates 100
> times.  That takes a long time, and it's only meant for cases where
> you're doing actual development and want high confidence.  The
> testsuite gives it an argument of 1, so that it iterates only once.
> That should ordinarily take only a few seconds.
> 
> Second, if you build with a GCC version before 4.7, it runs more
> slowly.  We can fix that but we haven't done it yet.  You can use a
> newer GCC version to make it run faster.
> 
> On Wed, Jul 16, 2014 at 10:16:16AM -0700, Daniele Di Proietto wrote:
>> The reason I didn?t do that in the first place is that test-cmap is already
>> taking 55s to run on my machine, and I thought that adding other code to check_cmap()
>> might increase runtime significantly (we call check_cmap() a lot and cmap_next_position()
>> is slow).
>> 
>> After applying your suggestion, it turns out this is not the case: it takes about 80s.
>> If you think this is an acceptable runtime (I think it?s ok), I?m about to send v2.
>> 
>> Daniele
>> 
>> On Jul 16, 2014, at 9:30 AM, Ben Pfaff <blp at nicira.com> wrote:
>> 
>>> On Tue, Jul 15, 2014 at 09:57:55PM -0700, Daniele Di Proietto wrote:
>>>> cmap_next_position() didn't update the node pointer while iterating through a
>>>> list of nodes with the same hash.
>>>> This commit fixes the bug and improve test-cmap to detect it.
>>>> 
>>>> Signed-off-by: Daniele Di Proietto <ddiproietto at vmware.com>
>>> 
>>> Good catch.
>>> 
>>> I think that it would be better to always try iterating both ways in
>>> check_cmap(), rather than just one way on each call.  Will you modify
>>> the patch to do that?
>>> 
>>> Thanks,
>>> 
>>> Ben
>> 




More information about the dev mailing list