On Sep 26, 2014, at 2:09 PM, Ben Pfaff <[email protected]> wrote:
> On Wed, Sep 24, 2014 at 11:24:02AM -0700, Jarno Rajahalme wrote:
>> Before:
>>
>> $ tests/ovstest test-bitmap benchmark 1000000
>> bitmap equal: 328 ms
>> bitmap scan: 8159 ms
>>
>> After:
>>
>> $ tests/ovstest test-bitmap benchmark 1000000
>> bitmap equal: 230 ms
>> bitmap scan: 185 ms
>>
>> Signed-off-by: Kmindg <[email protected]>
>> Signed-off-by: Jarno Rajahalme <[email protected]>
>
> Very nice.
>
> In bitmap_equal(), I think that this:
> i = ROUND_DOWN(n, BITMAP_ULONG_BITS);
> n -= i; /* Remaining bits. */
> can just be written "n %= BITMAP_ULONG_BITS;”
>
Thanks, it becomes even simpler this way.
> Did you consider writing a single bitmap scan function that takes a
> parameter to XOR with the bitmap words? Then bitmap_scan1() would
> pass 0 and bitmap_scan0() would pass ULONG_MAX. If this bitmap scan
> function were marked ALWAYS_INLINE (and the compiler honored it) then
> it would eliminate some redundant code.
>
You mean inlining bitmap_scan() (i.e., moving it to lib/bitmap.h, as otherwise
it would not be inlineable)? I just tested it , and it indeed is even faster.
If we do this, I’d be inclined to inline also the remaining functions in
bitmap.c, they are all trivial.
> In bitmap_scan[01], the inner "if (!unit)" seems to not be needed,
> because the outer start < end check will also do the right thing. I
> don't know whether it is better from a performance standpoint to
> remove the inner check, or whether it is better from an "obviously
> correct" standpoint to keep it.
>
The inner loop updates the ‘unit’, so it needs to be checked. However, it can
be written in a more obvious way like this:
...
for (; start < end; start += BITMAP_ULONG_BITS) {
unit = target ? *++p : ~*++p;
if (unit) {
goto found;
}
}
return end;
}
found:
...
> The comment on bitmap_init1() about its return value is wrong.
>
I had intended to change the function so that the operations can be chained, if
so desired.
> Acked-by: Ben Pfaff <[email protected]>
I’ll post a v2 in a moment,
Jarno
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
