Bug 36041 - Remove unnecessary differences in common code between WebCore::StringImpl & JSC::UStringImpl
Summary: Remove unnecessary differences in common code between WebCore::StringImpl & J...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore JavaScript (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Gavin Barraclough
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-03-11 17:42 PST by Gavin Barraclough
Modified: 2010-03-11 19:15 PST (History)
1 user (show)

See Also:


Attachments
Not setting r flag, abusing bugzilla to give me a pretty diff to get better comment per-function changes in my ChangeLog entries! (36.68 KB, patch)
2010-03-11 17:44 PST, Gavin Barraclough
no flags Details | Formatted Diff | Diff
The patch (37.75 KB, patch)
2010-03-11 18:26 PST, Gavin Barraclough
oliver: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Gavin Barraclough 2010-03-11 17:42:10 PST
Much of the code in WebCore::StringImpl and JSC::UStringImpl is now very similar, but has trivial and unnecessary formatting differences, such as the exact wording of comments, missing ASSERTs, functions implemented in the .h vs .cpp etc.
Comment 1 Gavin Barraclough 2010-03-11 17:44:19 PST
Created attachment 50564 [details]
Not setting r flag, abusing bugzilla to give me a pretty diff to get better comment per-function changes in my ChangeLog entries!
Comment 2 Gavin Barraclough 2010-03-11 18:26:50 PST
Created attachment 50569 [details]
The patch

Shows as a ~0.5% - 1% progression on SunSpider on my laptop.
This may be noise, through is repeatable & predominately seemed to come from moving the infrequently used UStringImpl::create() methods from the .h into the .cpp.
Comment 3 WebKit Review Bot 2010-03-11 18:29:29 PST
Attachment 50569 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
JavaScriptCore/runtime/UStringImpl.h:256:  More than one command on the same line  [whitespace/newline] [4]
JavaScriptCore/runtime/UStringImpl.h:256:  More than one command on the same line in if  [whitespace/parens] [4]
JavaScriptCore/runtime/UStringImpl.h:257:  More than one command on the same line  [whitespace/newline] [4]
WebCore/platform/text/StringImpl.h:170:  More than one command on the same line  [whitespace/newline] [4]
WebCore/platform/text/StringImpl.h:170:  More than one command on the same line in if  [whitespace/parens] [4]
Total errors found: 5 in 7 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 4 Oliver Hunt 2010-03-11 19:08:05 PST
Comment on attachment 50569 [details]
The patch

r=me
Comment 5 Gavin Barraclough 2010-03-11 19:15:01 PST
Comitted r55878