Replace % by bitwise &

• Component: Web Template Framework
• Status: NEW ---

[Rajeev Misra]

Replace % by bitwise &

[Rajeev Misra]

Created attachment 285895 [details]
Patch

[Alex Christensen]

Comment on attachment 285895 [details]
Patch

This needs a change log entry. Use Tools/Scripts/prepare-ChangeLog to generate a template

[Rajeev Misra]

Created attachment 285899 [details]
Patch

[Alexey Proskuryakov]

Comment on attachment 285899 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=285899&action=review

> Source/WTF/ChangeLog:8
> +        Replacing mod operation by bitwise &

Did you measure a performance improvement from this change?

[Rajeev Misra]

Comment on attachment 285899 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=285899&action=review

> Source/WTF/ChangeLog:9
> +        If we make size of HashTable as power

I tried to run TestWTF with "/usr/bin/time" to measure usr/sys/real times, but variation for 
different test runs made me conclude that it is not the reliable way to measure performance.  
This variation is probably because tests in TestWTF depends on scheduling order of different 
threads which might be different in each run. 

Is there any official tool/tests/process to measure performance in webkit?

I am not sure if there will be a immediate measurable performance improvement with 
this single change. 

This change was done with the thought that "If same result can be achieved with lesser 
cpu cycle, we should use that approach".  I am guessing division by "power of 2 number" 
done thru & uses lesser cpu cycle than division by some arbitrary number.

I think these smaller improvements could slowly adds up to give us better performance 
over time.

[Darin Adler]

Comment on attachment 285899 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=285899&action=review

>> Source/WTF/ChangeLog:9
>> +        If we make size of HashTable as power
> 
> I tried to run TestWTF with "/usr/bin/time" to measure usr/sys/real times, but variation for 
> different test runs made me conclude that it is not the reliable way to measure performance.  
> This variation is probably because tests in TestWTF depends on scheduling order of different 
> threads which might be different in each run. 
> 
> Is there any official tool/tests/process to measure performance in webkit?
> 
> I am not sure if there will be a immediate measurable performance improvement with 
> this single change. 
> 
> This change was done with the thought that "If same result can be achieved with lesser 
> cpu cycle, we should use that approach".  I am guessing division by "power of 2 number" 
> done thru & uses lesser cpu cycle than division by some arbitrary number.
> 
> I think these smaller improvements could slowly adds up to give us better performance 
> over time.

Sorry, that’s not how we do performance on this project. We don’t make changes based on a guess that something might be measurably faster. Further, I think your guess is likely incorrect. We do indeed have many real speedups that result from a collection many smaller improvements, but it is almost always many smaller *measurable* ones. Not changes that we guess are an improvement but can’t measure. There are lots of techniques to measure performance in WebKit but I doubt there is any that will be able to measure this. All the standard techniques, such as writing a targeted micro benchmark that uses a function in a tight loop. If making the table larger to avoid collisions is worth it, you might be able to prove that using collision statistics. But I don’t think any of that is going to work here; this really isn’t a worthwhile improvement. It makes the code harder to read and almost certainly does not provide a commensurate speedup.

I think you should choose a more valuable project for your first WebKit patch, perhaps a bug fix. Do you have any other areas of interest?

> Source/WTF/wtf/ParkingLot.cpp:220
> +        int leadingZeros = size > 1 ? __builtin_clz(size - 1) : __builtin_clz(size);

Does not compile on Windows because __builtin_clz is not supported on that platform.