Summary: | [V8] Fix a fixme in v8 bindings | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Erik Arvidsson <arv> | ||||||||||
Component: | New Bugs | Assignee: | Erik Arvidsson <arv> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | abarth, haraken, japhet, webkit.review.bot | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Attachments: |
|
Description
Erik Arvidsson
2012-03-09 14:58:43 PST
Created attachment 131119 [details]
Patch
toUInt32 and toInt32 are not used in normal DOM code paths so it is not very hot. I see WebGL related methods using it so I expect that this might have some benefit there. Comment on attachment 131119 [details]
Patch
This change looks good in that it removes FIXME. But basically I wanted to see some performance tests (e.g. "XXX becomes YYY% faster") if you intend to make some performance optimization (i.e. inlining). We are likely to make wasteful "optimization".
I made a quick test with ArrayBuffer and the inlining had no effect. Uploading new patch in a second Created attachment 131129 [details]
Patch for landing
Created attachment 131131 [details]
Patch for landing
Attachment 131131 [details] did not pass style-queue:
Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/bindings/v8/V8Binding.cpp']" exit_code: 1
Source/WebCore/bindings/v8/V8Binding.cpp:246: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5]
Total errors found: 1 in 1 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 131133 [details]
Patch
Comment on attachment 131133 [details] Patch Clearing flags on attachment: 131133 Committed r110369: <http://trac.webkit.org/changeset/110369> All reviewed patches have been landed. Closing bug. |