WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
NEW
160799
Replace % by bitwise &
https://bugs.webkit.org/show_bug.cgi?id=160799
Summary
Replace % by bitwise &
Rajeev Misra
Reported
2016-08-11 22:22:08 PDT
Replace % by bitwise &
Attachments
Patch
(2.05 KB, patch)
2016-08-11 22:27 PDT
,
Rajeev Misra
no flags
Details
Formatted Diff
Diff
Patch
(2.99 KB, patch)
2016-08-11 23:33 PDT
,
Rajeev Misra
darin
: review-
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Rajeev Misra
Comment 1
2016-08-11 22:27:02 PDT
Created
attachment 285895
[details]
Patch
Alex Christensen
Comment 2
2016-08-11 23:17:47 PDT
Comment on
attachment 285895
[details]
Patch This needs a change log entry. Use Tools/Scripts/prepare-ChangeLog to generate a template
Rajeev Misra
Comment 3
2016-08-11 23:33:54 PDT
Created
attachment 285899
[details]
Patch
Alexey Proskuryakov
Comment 4
2016-08-12 07:30:45 PDT
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 5
2016-08-12 09:45:20 PDT
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 6
2016-08-15 23:45:43 PDT
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.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug