Bug 182563

Summary: Experiment with alternative implementation of memcpy/memset
Product: WebKit Reporter: Filip Pizlo <fpizlo>
Component: Web Template FrameworkAssignee: Filip Pizlo <fpizlo>
Status: REOPENED    
Severity: Normal CC: benjamin, bugs-noreply, cdumez, clopez, cmarcelo, dbates, ews-watchlist, jlewis3, keith_miller, mark.lam, mcatanzaro, msaboff, saam, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Attachments:
Description Flags
work in progress
none
the patch
none
the patch
msaboff: review+
patch for landing none

Filip Pizlo
Reported 2018-02-06 20:04:17 PST
...
Attachments
work in progress (54.06 KB, patch)
2018-02-06 20:05 PST, Filip Pizlo 		no flags
DetailsFormatted DiffDiff
the patch (66.31 KB, patch)
2018-02-08 10:08 PST, Filip Pizlo 		no flags
DetailsFormatted DiffDiff
the patch (68.98 KB, patch)
2018-02-08 10:20 PST, Filip Pizlo 		msaboff: review+
DetailsFormatted DiffDiff
patch for landing (68.72 KB, patch)
2018-02-08 12:57 PST, Filip Pizlo 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Filip Pizlo
Comment 1 2018-02-06 20:05:22 PST
Created attachment 333260 [details] work in progress
Filip Pizlo
Comment 2 2018-02-07 15:50:28 PST
Looks like a 1.00882x speed-up on PLT with p = 0.0973514. Gonna test some more benchmarks.
Filip Pizlo
Comment 3 2018-02-08 10:08:42 PST
Created attachment 333383 [details] the patch
EWS Watchlist
Comment 4 2018-02-08 10:11:46 PST
Attachment 333383 [details] did not pass style-queue: ERROR: Source/bmalloc/bmalloc/Algorithm.h:190: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:203: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:210: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:231: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:238: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:259: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:285: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:290: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:295: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:285: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:290: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:295: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:285: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:290: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:295: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:265: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:285: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:290: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:295: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:290: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:295: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:290: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:295: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:301: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:313: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:314: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:314: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:320: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:339: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:340: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:340: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:346: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:365: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:366: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:389: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:390: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:394: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:366: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:389: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:390: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:394: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:389: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:390: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:394: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:371: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:389: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:390: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:394: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:390: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:394: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:394: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:39: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:51: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:52: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:52: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:58: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:77: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:78: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:78: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:84: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:103: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:104: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:127: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:128: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:132: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:136: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:104: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:127: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:128: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:132: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:136: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:127: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:128: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:132: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:136: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:109: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:127: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:128: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:132: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:136: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:128: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:132: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:136: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:132: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:136: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:39: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WTF/wtf/FastCopy.h:52: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:59: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WTF/wtf/FastCopy.h:80: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:87: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WTF/wtf/FastCopy.h:108: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:134: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:139: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:143: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:134: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:139: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:143: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:134: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:139: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:143: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:114: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WTF/wtf/FastCopy.h:134: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:139: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:143: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:139: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:143: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:139: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:143: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] Total errors found: 108 in 41 files If any of these errors are false positives, please file a bug against check-webkit-style.
Filip Pizlo
Comment 5 2018-02-08 10:20:15 PST
Created attachment 333386 [details] the patch
EWS Watchlist
Comment 6 2018-02-08 10:23:07 PST
Attachment 333386 [details] did not pass style-queue: ERROR: Source/bmalloc/bmalloc/Algorithm.h:190: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:203: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:210: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:231: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:238: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:259: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:285: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:290: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:295: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:285: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:290: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:295: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:285: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:290: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:295: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:265: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:285: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:290: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:295: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:290: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:295: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:290: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:295: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:301: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:313: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:314: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:314: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:320: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:339: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:340: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:340: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:346: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:365: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:366: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:389: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:390: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:394: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:366: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:389: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:390: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:394: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:389: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:390: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:394: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:371: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:389: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:390: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:394: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:390: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:394: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:394: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:39: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:51: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:52: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:52: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:58: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:77: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:78: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:78: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:84: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:103: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:104: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:127: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:128: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:132: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:136: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:104: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:127: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:128: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:132: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:136: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:127: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:128: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:132: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:136: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:109: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:127: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:128: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:132: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:136: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:128: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:132: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:136: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:132: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:136: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:39: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WTF/wtf/FastCopy.h:52: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:59: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WTF/wtf/FastCopy.h:80: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:87: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WTF/wtf/FastCopy.h:108: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:134: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:139: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:143: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:134: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:139: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:143: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:134: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:139: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:143: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:114: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WTF/wtf/FastCopy.h:134: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:139: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:143: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:139: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:143: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:139: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:143: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] Total errors found: 108 in 43 files If any of these errors are false positives, please file a bug against check-webkit-style.
Michael Saboff
Comment 7 2018-02-08 11:21:32 PST
Comment on attachment 333386 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=333386&action=review r=me. Did you consider implementing only in bmalloc and then a forward reference in WTF? > Source/WTF/ChangeLog:36 > + reasoning. But that's not quite right, since then we would lose inlining in the unknonw-size Spelling *unknonw-size*
Mark Lam
Comment 8 2018-02-08 12:20:30 PST
Comment on attachment 333386 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=333386&action=review FYI, I did my review from the bottom up (since the fastCopy and fastZeroFill implementations are located towards the bottom). It might make more sense to read my comments backwards. > Source/WTF/wtf/FastCopy.h:33 > +template<typename T> > +void fastCopy(T* dst, const T* src, size_t length) I assume this is identical to the bmalloc version below and didn't review this thoroughly. > Source/WTF/wtf/FastCopy.h:143 > +inline void fastCopyBytes(void* dst, const void* src, size_t bytes) At first, I was pondering whether we can rename this to just be fastCopy. After doing the entire review, I am strongly of the opinion that we should NOT rename this to fastCopy. There's a strong need to distinguish fastCopy and fastCopyBytes apart. One of the class of errors that can arise from fastCopy() taking a count of T to copy instead of size_t bytes is that if someone edits the code later and innocently changes the type of the pointer, an error may silently creep in. Consider this scenario: Initially, let's say we have this contrived code in a class: void foo(Stuff* myStuff, Stuff* other, int numberOfStuff) { m_myStuff = myStuff; m_other = other; m_length = numberOfStuff; bar(); // where bar contains ... fastCopy(m_myStuff, m_other, m_length); // <======== this is correct. } Stuff m_myStuff; Stuff m_other; size_t m_length; Some time later, for reasons, someone decides to change type of the pointers like so: void* m_myStuff; void* m_other; size_t m_length; The person making the change may not notice that bar() relies on typeof m_myStuff and m_other not being void*. If you can be sure that calling fastCopy() with void* pointers will result in a build error, then this error will not go unnoticed. I think we're fine, but can you please do a quick test to make sure that this is indeed the case i.e. we should get build errors if fastCopy() is called with void* pointers. This is also why it is good to name fastCopyBytes() differently than fastCopy(). > Source/WTF/wtf/FastMalloc.cpp:83 > + fastZeroFill(static_cast<char*>(result), n); Let's just call fastZeroFillBytes() and do away with the cast instead. > Source/WTF/wtf/FastMalloc.cpp:100 > + fastZeroFill(static_cast<char*>(result), n); Let's just call fastZeroFillBytes() and do away with the cast instead. > Source/WTF/wtf/FastZeroFill.h:33 > +void fastZeroFill(T* dst, size_t length) I assume this is identical to the bmalloc version below and didn't review this thoroughly. > Source/WTF/wtf/FastZeroFill.h:136 > +inline void fastZeroFillBytes(void* dst, size_t bytes) Ditto: same issue as with fastCopyBytes() above. > Source/WTF/wtf/MD5.cpp:5 > - * Copyright (C) 2015 Apple Inc. All rights reserved. > + * Copyright (C) 2015-2018 Apple Inc. All rights reserved. Why is this needed? > Source/WTF/wtf/text/StringConcatenate.h:161 > + fastCopy(destination, m_characters, m_length * sizeof(UChar)); This is wrong. You should remove the "* sizeof(UChar)". > Source/bmalloc/bmalloc/Algorithm.h:218 > + "movq (%%rsi, %%rcx), %%rax\n\t" > + "movq %%rax, (%%rdi, %%rcx)\n\t" Ditto with assumption about alignment being a non-issue (see below). > Source/bmalloc/bmalloc/Algorithm.h:229 > + "addq $4, %%rcx\n\t" > + "4:\n\t" > + "movl (%%rsi, %%rcx), %%eax\n\t" > + "movl %%eax, (%%rdi, %%rcx)\n\t" > + "subq $4, %%rcx\n\t" > + "jae 4b\n\t" Same as below: no need for addq, subq, and jae. This reduces to: "movl (%%rsi), %%eax\n\t" "movl %%eax, (%%rdi)\n\t" > Source/bmalloc/bmalloc/Algorithm.h:246 > + "movq (%%rsi, %%rcx), %%rax\n\t" > + "movq %%rax, (%%rdi, %%rcx)\n\t" Ditto with assumption about alignment being a non-issue (see below). > Source/bmalloc/bmalloc/Algorithm.h:272 > + "movq (%%rsi, %%rcx), %%rax\n\t" > + "movq %%rax, (%%rdi, %%rcx)\n\t" Ditto with assumption about alignment being a non-issue (see below). > Source/bmalloc/bmalloc/Algorithm.h:324 > + "rep stosl\n\t" I presume the CPU's implementation of stosl will do the equivalent of stosq for the parts that are 8 byte aligned? Can you confirm? > Source/bmalloc/bmalloc/Algorithm.h:330 > + "1:\n\t" > + "subq $8, %%rcx\n\t" > + "jae 3b\n\t" It looks like you're assuming that fastZeroFill() will only every be called on a dst that is aligned on a 8 byte boundary because you're not checking for the potential of a leading 4 bytes here. Or are you relying on x86 support for unaligned access and just taking the hit if it turns out to be unaligned? > Source/bmalloc/bmalloc/Algorithm.h:337 > + "addq $4, %%rcx\n\t" > + "4:\n\t" > + "movl %%eax, (%%rdi, %%rcx)\n\t" > + "subq $4, %%rcx\n\t" > + "jae 4b\n\t" We already know that sizeof(T) % sizeof(uint32_t) == 0. Hence, the only way that we can get here (%rcx != -8) is if there's one 4 byte word to write. I think you can remove this addq, subq, and jae. This reduces to: "movl %%eax, (%%rdi)\n\t" > Source/bmalloc/bmalloc/Algorithm.h:353 > + "movq %%rax, (%%rdi, %%rcx)\n\t" Ditto with assumption about alignment being a non-issue. > Source/bmalloc/bmalloc/Algorithm.h:377 > + "movq %%rax, (%%rdi, %%rcx)\n\t" Ditto with assumption about alignment being a non-issue.
Filip Pizlo
Comment 9 2018-02-08 12:49:50 PST
(In reply to Mark Lam from comment #8) > Comment on attachment 333386 [details] > the patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=333386&action=review > > FYI, I did my review from the bottom up (since the fastCopy and fastZeroFill > implementations are located towards the bottom). It might make more sense > to read my comments backwards. > > > Source/WTF/wtf/FastCopy.h:33 > > +template<typename T> > > +void fastCopy(T* dst, const T* src, size_t length) > > I assume this is identical to the bmalloc version below and didn't review > this thoroughly. > > > Source/WTF/wtf/FastCopy.h:143 > > +inline void fastCopyBytes(void* dst, const void* src, size_t bytes) > > At first, I was pondering whether we can rename this to just be fastCopy. > After doing the entire review, I am strongly of the opinion that we should > NOT rename this to fastCopy. There's a strong need to distinguish fastCopy > and fastCopyBytes apart. > > One of the class of errors that can arise from fastCopy() taking a count of > T to copy instead of size_t bytes is that if someone edits the code later > and innocently changes the type of the pointer, an error may silently creep > in. Consider this scenario: > > Initially, let's say we have this contrived code in a class: > > void foo(Stuff* myStuff, Stuff* other, int numberOfStuff) { > m_myStuff = myStuff; > m_other = other; > m_length = numberOfStuff; > bar(); // where bar contains ... fastCopy(m_myStuff, m_other, > m_length); // <======== this is correct. > } > > Stuff m_myStuff; > Stuff m_other; > size_t m_length; > > Some time later, for reasons, someone decides to change type of the pointers > like so: > > void* m_myStuff; > void* m_other; > size_t m_length; > > The person making the change may not notice that bar() relies on typeof > m_myStuff and m_other not being void*. If you can be sure that calling > fastCopy() with void* pointers will result in a build error, then this error > will not go unnoticed. I think we're fine, but can you please do a quick > test to make sure that this is indeed the case i.e. we should get build > errors if fastCopy() is called with void* pointers. We do not get build errors if fastCopy() is called with void*. > > This is also why it is good to name fastCopyBytes() differently than > fastCopy(). > > > Source/WTF/wtf/FastMalloc.cpp:83 > > + fastZeroFill(static_cast<char*>(result), n); > > Let's just call fastZeroFillBytes() and do away with the cast instead. Fixed. > > > Source/WTF/wtf/FastMalloc.cpp:100 > > + fastZeroFill(static_cast<char*>(result), n); > > Let's just call fastZeroFillBytes() and do away with the cast instead. Fixed. > > > Source/WTF/wtf/FastZeroFill.h:33 > > +void fastZeroFill(T* dst, size_t length) > > I assume this is identical to the bmalloc version below and didn't review > this thoroughly. Yeah. > > > Source/WTF/wtf/FastZeroFill.h:136 > > +inline void fastZeroFillBytes(void* dst, size_t bytes) > > Ditto: same issue as with fastCopyBytes() above. Yeah. > > > Source/WTF/wtf/MD5.cpp:5 > > - * Copyright (C) 2015 Apple Inc. All rights reserved. > > + * Copyright (C) 2015-2018 Apple Inc. All rights reserved. > > Why is this needed? Reverted. An earlier version replaced memcpy/memset in this file, but later I noticed it was being used to copy constant-size things, so I reverted. > > > Source/WTF/wtf/text/StringConcatenate.h:161 > > + fastCopy(destination, m_characters, m_length * sizeof(UChar)); > > This is wrong. You should remove the "* sizeof(UChar)". Fixed! > > > Source/bmalloc/bmalloc/Algorithm.h:218 > > + "movq (%%rsi, %%rcx), %%rax\n\t" > > + "movq %%rax, (%%rdi, %%rcx)\n\t" > > Ditto with assumption about alignment being a non-issue (see below). I confirmed that alignment is a non-issue! It appears that doing a misaligned 64-bit word load/store is always faster than doing 8 8-bit loads/stores. Also, other memcpy's that I've seen for x86 do the same thing. > > > Source/bmalloc/bmalloc/Algorithm.h:229 > > + "addq $4, %%rcx\n\t" > > + "4:\n\t" > > + "movl (%%rsi, %%rcx), %%eax\n\t" > > + "movl %%eax, (%%rdi, %%rcx)\n\t" > > + "subq $4, %%rcx\n\t" > > + "jae 4b\n\t" > > Same as below: no need for addq, subq, and jae. This reduces to: > "movl (%%rsi), %%eax\n\t" > "movl %%eax, (%%rdi)\n\t" Clever. I'll leave a comment and fix in another patch: https://bugs.webkit.org/show_bug.cgi?id=182617 > > > Source/bmalloc/bmalloc/Algorithm.h:246 > > + "movq (%%rsi, %%rcx), %%rax\n\t" > > + "movq %%rax, (%%rdi, %%rcx)\n\t" > > Ditto with assumption about alignment being a non-issue (see below). Will leave fixme. > > > Source/bmalloc/bmalloc/Algorithm.h:272 > > + "movq (%%rsi, %%rcx), %%rax\n\t" > > + "movq %%rax, (%%rdi, %%rcx)\n\t" > > Ditto with assumption about alignment being a non-issue (see below). Will leave fixme. > > > Source/bmalloc/bmalloc/Algorithm.h:324 > > + "rep stosl\n\t" > > I presume the CPU's implementation of stosl will do the equivalent of stosq > for the parts that are 8 byte aligned? Can you confirm? > > > Source/bmalloc/bmalloc/Algorithm.h:330 > > + "1:\n\t" > > + "subq $8, %%rcx\n\t" > > + "jae 3b\n\t" > > It looks like you're assuming that fastZeroFill() will only every be called > on a dst that is aligned on a 8 byte boundary because you're not checking > for the potential of a leading 4 bytes here. Or are you relying on x86 > support for unaligned access and just taking the hit if it turns out to be > unaligned? Relying on x86 to be awesome. ;-) > > > Source/bmalloc/bmalloc/Algorithm.h:337 > > + "addq $4, %%rcx\n\t" > > + "4:\n\t" > > + "movl %%eax, (%%rdi, %%rcx)\n\t" > > + "subq $4, %%rcx\n\t" > > + "jae 4b\n\t" > > We already know that sizeof(T) % sizeof(uint32_t) == 0. Hence, the only way > that we can get here (%rcx != -8) is if there's one 4 byte word to write. I > think you can remove this addq, subq, and jae. This reduces to: > "movl %%eax, (%%rdi)\n\t" That's a really good observation. https://bugs.webkit.org/show_bug.cgi?id=182617 > > > Source/bmalloc/bmalloc/Algorithm.h:353 > > + "movq %%rax, (%%rdi, %%rcx)\n\t" > > Ditto with assumption about alignment being a non-issue. Not an issue. > > > Source/bmalloc/bmalloc/Algorithm.h:377 > > + "movq %%rax, (%%rdi, %%rcx)\n\t" > > Ditto with assumption about alignment being a non-issue. Not an issue.
Filip Pizlo
Comment 10 2018-02-08 12:52:00 PST
(In reply to Filip Pizlo from comment #9) > (In reply to Mark Lam from comment #8) > > Comment on attachment 333386 [details] > > the patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=333386&action=review > > > > FYI, I did my review from the bottom up (since the fastCopy and fastZeroFill > > implementations are located towards the bottom). It might make more sense > > to read my comments backwards. > > > > > Source/WTF/wtf/FastCopy.h:33 > > > +template<typename T> > > > +void fastCopy(T* dst, const T* src, size_t length) > > > > I assume this is identical to the bmalloc version below and didn't review > > this thoroughly. > > > > > Source/WTF/wtf/FastCopy.h:143 > > > +inline void fastCopyBytes(void* dst, const void* src, size_t bytes) > > > > At first, I was pondering whether we can rename this to just be fastCopy. > > After doing the entire review, I am strongly of the opinion that we should > > NOT rename this to fastCopy. There's a strong need to distinguish fastCopy > > and fastCopyBytes apart. > > > > One of the class of errors that can arise from fastCopy() taking a count of > > T to copy instead of size_t bytes is that if someone edits the code later > > and innocently changes the type of the pointer, an error may silently creep > > in. Consider this scenario: > > > > Initially, let's say we have this contrived code in a class: > > > > void foo(Stuff* myStuff, Stuff* other, int numberOfStuff) { > > m_myStuff = myStuff; > > m_other = other; > > m_length = numberOfStuff; > > bar(); // where bar contains ... fastCopy(m_myStuff, m_other, > > m_length); // <======== this is correct. > > } > > > > Stuff m_myStuff; > > Stuff m_other; > > size_t m_length; > > > > Some time later, for reasons, someone decides to change type of the pointers > > like so: > > > > void* m_myStuff; > > void* m_other; > > size_t m_length; > > > > The person making the change may not notice that bar() relies on typeof > > m_myStuff and m_other not being void*. If you can be sure that calling > > fastCopy() with void* pointers will result in a build error, then this error > > will not go unnoticed. I think we're fine, but can you please do a quick > > test to make sure that this is indeed the case i.e. we should get build > > errors if fastCopy() is called with void* pointers. > > We do not get build errors if fastCopy() is called with void*. I was wrong. We totally do, because of sizeof(void).
Filip Pizlo
Comment 11 2018-02-08 12:57:04 PST
Created attachment 333413 [details] patch for landing
EWS Watchlist
Comment 12 2018-02-08 12:59:56 PST
Attachment 333413 [details] did not pass style-queue: ERROR: Source/bmalloc/bmalloc/Algorithm.h:190: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:203: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:210: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:231: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:238: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:259: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:285: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:290: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:295: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:285: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:290: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:295: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:285: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:290: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:295: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:265: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:285: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:290: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:295: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:290: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:295: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:290: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:295: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:301: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:313: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:314: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:314: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:320: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:339: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:340: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:340: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:346: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:365: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:366: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:389: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:390: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:394: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:366: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:389: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:390: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:394: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:389: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:390: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:394: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:371: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:389: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:390: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:394: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:390: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:394: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/bmalloc/bmalloc/Algorithm.h:394: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:39: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:51: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:52: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:52: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:58: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:77: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:78: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:78: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:84: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:103: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:104: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:127: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:128: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:132: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:136: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:104: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:127: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:128: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:132: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:136: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:127: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:128: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:132: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:136: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:109: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:127: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:128: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:132: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:136: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:128: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:132: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:136: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:132: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastZeroFill.h:136: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:39: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WTF/wtf/FastCopy.h:52: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:59: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WTF/wtf/FastCopy.h:80: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:87: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WTF/wtf/FastCopy.h:108: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:134: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:139: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:143: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:134: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:139: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:143: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:134: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:139: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:143: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:114: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WTF/wtf/FastCopy.h:134: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:139: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:143: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:139: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:143: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:139: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/FastCopy.h:143: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] Total errors found: 108 in 42 files If any of these errors are false positives, please file a bug against check-webkit-style.
Mark Lam
Comment 13 2018-02-08 13:01:44 PST
(In reply to Filip Pizlo from comment #10) > (In reply to Filip Pizlo from comment #9) > > We do not get build errors if fastCopy() is called with void*. > > I was wrong. We totally do, because of sizeof(void). Excellent.
Mark Lam
Comment 14 2018-02-08 13:04:52 PST
Comment on attachment 333413 [details] patch for landing View in context: https://bugs.webkit.org/attachment.cgi?id=333413&action=review > Source/WTF/wtf/FastCopy.h:101 > + "addq $6, %%rcx\n\t" // FIXME: This isn't really a loop. https://bugs.webkit.org/show_bug.cgi?id=182617 Wrong place. This comment belongs on the uint32_t case above. > Source/WTF/wtf/FastZeroFill.h:97 > + "addq $6, %%rcx\n\t" // FIXME: This isn't really a loop. https://bugs.webkit.org/show_bug.cgi?id=182617 Wrong place. This comment belongs on the uint32_t case above. > Source/bmalloc/bmalloc/Algorithm.h:252 > + "addq $6, %%rcx\n\t" // FIXME: This isn't really a loop. https://bugs.webkit.org/show_bug.cgi?id=182617 Wrong place. This comment belongs on the uint32_t case above. > Source/bmalloc/bmalloc/Algorithm.h:359 > + "addq $6, %%rcx\n\t" // FIXME: This isn't really a loop. https://bugs.webkit.org/show_bug.cgi?id=182617 Wrong place. This comment belongs on the uint32_t case above.
Filip Pizlo
Comment 15 2018-02-08 18:11:45 PST
(In reply to Mark Lam from comment #14) > Comment on attachment 333413 [details] > patch for landing > > View in context: > https://bugs.webkit.org/attachment.cgi?id=333413&action=review > > > Source/WTF/wtf/FastCopy.h:101 > > + "addq $6, %%rcx\n\t" // FIXME: This isn't really a loop. https://bugs.webkit.org/show_bug.cgi?id=182617 > > Wrong place. This comment belongs on the uint32_t case above. > > > Source/WTF/wtf/FastZeroFill.h:97 > > + "addq $6, %%rcx\n\t" // FIXME: This isn't really a loop. https://bugs.webkit.org/show_bug.cgi?id=182617 > > Wrong place. This comment belongs on the uint32_t case above. > > > Source/bmalloc/bmalloc/Algorithm.h:252 > > + "addq $6, %%rcx\n\t" // FIXME: This isn't really a loop. https://bugs.webkit.org/show_bug.cgi?id=182617 > > Wrong place. This comment belongs on the uint32_t case above. > > > Source/bmalloc/bmalloc/Algorithm.h:359 > > + "addq $6, %%rcx\n\t" // FIXME: This isn't really a loop. https://bugs.webkit.org/show_bug.cgi?id=182617 > > Wrong place. This comment belongs on the uint32_t case above. Fixed!
Filip Pizlo
Comment 16 2018-02-08 18:13:45 PST
Landed in http://trac.webkit.org/changeset/228306/webkit Saam and I will watch the bots to see if this is really a speed-up. It's hard to tell. When I retested PLT, I didn't see anything. But a Speedometer run did see a maybe-speed-up. So who knows!
Filip Pizlo
Comment 17 2018-02-08 18:14:01 PST
Landed in http://trac.webkit.org/changeset/228306/webkit
Radar WebKit Bug Importer
Comment 18 2018-02-08 18:14:27 PST
<rdar://problem/37374970>
Carlos Alberto Lopez Perez
Comment 20 2018-02-09 05:37:49 PST
Committed r228317: <https://trac.webkit.org/changeset/228317>
Carlos Alberto Lopez Perez
Comment 21 2018-02-09 05:50:47 PST
Committed r228318: <https://trac.webkit.org/changeset/228318>
Michael Catanzaro
Comment 22 2018-02-09 08:07:32 PST
(In reply to Carlos Alberto Lopez Perez from comment #20) > Committed r228317: <https://trac.webkit.org/changeset/228317> We should avoid architecture-specific #includes whenever possible. Adding one #include <string.h> is not going to slow down the build very much at all; it's not worth the cost of the include guards, IMO.
Carlos Alberto Lopez Perez
Comment 23 2018-02-09 09:04:31 PST
(In reply to Michael Catanzaro from comment #22) > (In reply to Carlos Alberto Lopez Perez from comment #20) > > Committed r228317: <https://trac.webkit.org/changeset/228317> > > We should avoid architecture-specific #includes whenever possible. Adding > one #include <string.h> is not going to slow down the build very much at > all; it's not worth the cost of the include guards, IMO. Thanks for the suggestion. I have uploaded a patch for this to bug 182642
Matt Lewis
Comment 24 2018-02-16 14:06:37 PST
Reverted r228318 for reason: The patch that this attempted to fix was rolled out already. Committed r228582: <https://trac.webkit.org/changeset/228582>
Note You need to log in before you can comment on or make changes to this bug.