RESOLVED FIXED 157045
Speed up JSGlobalObject initialization by making some properties lazy
https://bugs.webkit.org/show_bug.cgi?id=157045
Summary Speed up JSGlobalObject initialization by making some properties lazy
Filip Pizlo
Reported 2016-04-26 14:54:11 PDT
Patch forthcoming.
Attachments
work in progress (6.26 KB, patch)
2016-04-26 14:54 PDT, Filip Pizlo
no flags
a bit more (13.39 KB, patch)
2016-04-26 17:03 PDT, Filip Pizlo
no flags
some more (27.83 KB, patch)
2016-04-26 21:30 PDT, Filip Pizlo
no flags
it's getting weird (63.61 KB, patch)
2016-04-27 14:55 PDT, Filip Pizlo
no flags
moar (81.04 KB, patch)
2016-04-27 20:50 PDT, Filip Pizlo
no flags
getting close (83.51 KB, patch)
2016-04-27 22:09 PDT, Filip Pizlo
no flags
it compiles a bit more (104.11 KB, patch)
2016-04-28 12:31 PDT, Filip Pizlo
no flags
OMG JSGlobalObject.cpp compiles (114.58 KB, patch)
2016-04-28 14:25 PDT, Filip Pizlo
no flags
2x faster to create JSGlobalObject (123.27 KB, patch)
2016-04-28 15:18 PDT, Filip Pizlo
no flags
less wrong (124.42 KB, patch)
2016-04-28 15:46 PDT, Filip Pizlo
no flags
totally awesome (144.61 KB, patch)
2016-04-29 16:18 PDT, Filip Pizlo
no flags
rebased (145.26 KB, patch)
2016-04-29 17:13 PDT, Filip Pizlo
no flags
with fixes (144.58 KB, patch)
2016-04-29 18:01 PDT, Filip Pizlo
no flags
with more fixes (144.22 KB, patch)
2016-04-30 12:07 PDT, Filip Pizlo
no flags
and now, even less broken! (144.70 KB, patch)
2016-04-30 12:12 PDT, Filip Pizlo
no flags
less broken (144.85 KB, patch)
2016-04-30 12:23 PDT, Filip Pizlo
no flags
Archive of layout-test-results from ews114 for mac-yosemite (348.47 KB, application/zip)
2016-04-30 13:17 PDT, Build Bot
no flags
Archive of layout-test-results from ews103 for mac-yosemite (1.41 MB, application/zip)
2016-04-30 13:28 PDT, Build Bot
no flags
Archive of layout-test-results from ews107 for mac-yosemite-wk2 (1.23 MB, application/zip)
2016-04-30 13:38 PDT, Build Bot
no flags
Archive of layout-test-results from ews126 for ios-simulator-wk2 (747.08 KB, application/zip)
2016-04-30 13:38 PDT, Build Bot
no flags
more (148.98 KB, patch)
2016-05-01 11:54 PDT, Filip Pizlo
buildbot: commit-queue-
Archive of layout-test-results from ews113 for mac-yosemite (566.15 KB, application/zip)
2016-05-01 12:52 PDT, Build Bot
no flags
maybe it's done now (154.33 KB, patch)
2016-05-01 15:10 PDT, Filip Pizlo
no flags
it's more done than ever before! (155.49 KB, patch)
2016-05-01 16:38 PDT, Filip Pizlo
no flags
even more fixes (158.49 KB, patch)
2016-05-01 16:57 PDT, Filip Pizlo
no flags
even more fixes (158.51 KB, patch)
2016-05-01 18:15 PDT, Filip Pizlo
no flags
removed some Heap.cpp debugging hacks (157.67 KB, patch)
2016-05-01 19:01 PDT, Filip Pizlo
buildbot: commit-queue-
Archive of layout-test-results from ews116 for mac-yosemite (1.32 MB, application/zip)
2016-05-01 20:17 PDT, Build Bot
no flags
even better (161.56 KB, patch)
2016-05-01 21:16 PDT, Filip Pizlo
no flags
even betterer (161.80 KB, patch)
2016-05-02 10:19 PDT, Filip Pizlo
no flags
rebased (164.44 KB, patch)
2016-05-02 11:54 PDT, Filip Pizlo
keith_miller: review+
patch for relanding (168.87 KB, patch)
2016-05-04 13:08 PDT, Filip Pizlo
no flags
patch for relanding (169.28 KB, patch)
2016-05-04 13:31 PDT, Filip Pizlo
no flags
Filip Pizlo
Comment 1 2016-04-26 14:54:56 PDT
Created attachment 277410 [details] work in progress
Filip Pizlo
Comment 2 2016-04-26 17:03:48 PDT
Created attachment 277424 [details] a bit more I'm starting to have a plan.
Filip Pizlo
Comment 3 2016-04-26 21:29:28 PDT
I'm having a lot of fun with this. It took me a while to figure out the right idioms. I believe that we can make most of the things in JSGlobalObject lazy. I'm using a combination of the following two approaches. - LazyProperty<>. This is like WriteBarrier<> but it allows you to initialize it lazily. You can do: m_myLazyProperty.initLater([] (const Initializer& init) { init.set(...); }); You're required to use a stateless (i.e. "[]") lambda, and we will assert this. It successfully converts the lambda into a function pointer and stuffs it into itself. Then later when you call get(), it will invoke this lambda by extracting it from the function pointer. Crucially, the initLater() method compiles down to a store of a constant function pointer into the pointer field inside m_myLazyProperty. - New Lookup attributes. It's now possible to put various kinds of property creation callbacks in a Lookup hashtable. This can be married to a LazyProperty<> so that the laziness of initialization is coordinated between the two. When this is all put together, it should be easy to write code in JSGlobalObject that lazily constructs everything. I'm playing with this now.
Filip Pizlo
Comment 4 2016-04-26 21:30:48 PDT
Created attachment 277440 [details] some more
Filip Pizlo
Comment 5 2016-04-27 14:55:08 PDT
Created attachment 277539 [details] it's getting weird But it will be even weirder soon.
Filip Pizlo
Comment 6 2016-04-27 20:50:42 PDT
Filip Pizlo
Comment 7 2016-04-27 22:09:34 PDT
Created attachment 277590 [details] getting close Still need to change all of the getters and the visitChildren. Then I might be done.
Filip Pizlo
Comment 8 2016-04-28 12:31:49 PDT
Created attachment 277640 [details] it compiles a bit more Still lots of compiler errors though
Filip Pizlo
Comment 9 2016-04-28 14:25:48 PDT
Created attachment 277649 [details] OMG JSGlobalObject.cpp compiles Wow
Filip Pizlo
Comment 10 2016-04-28 15:18:36 PDT
Created attachment 277654 [details] 2x faster to create JSGlobalObject I'm sure it's still completely broken, but it is 2x faster on my microbenchmark.
Filip Pizlo
Comment 11 2016-04-28 15:19:26 PDT
*** Bug 157044 has been marked as a duplicate of this bug. ***
Filip Pizlo
Comment 12 2016-04-28 15:46:18 PDT
Created attachment 277656 [details] less wrong Still dealing with basics.
Filip Pizlo
Comment 13 2016-04-29 16:18:54 PDT
Created attachment 277752 [details] totally awesome It passes tests!
Filip Pizlo
Comment 14 2016-04-29 17:13:29 PDT
WebKit Commit Bot
Comment 15 2016-04-29 17:16:12 PDT
Attachment 277762 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/runtime/LazyProperty.h:103: The parameter name "initializer" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/runtime/LazyClassStructure.h:45: The parameter name "prototype" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/runtime/LazyClassStructure.h:59: The parameter name "constructor" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WTF/wtf/StdLibExtras.h:67: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:344: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:348: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:353: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:357: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:367: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:371: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:377: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:385: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:389: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:403: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:410: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:429: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:437: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:442: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:446: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:451: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:456: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:463: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:467: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:473: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:477: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:577: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:582: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:586: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:591: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/LazyClassStructureInlines.h:38: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/heap/Heap.cpp:1132: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.h:34: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 32 in 34 files If any of these errors are false positives, please file a bug against check-webkit-style.
Filip Pizlo
Comment 16 2016-04-29 18:01:15 PDT
Created attachment 277770 [details] with fixes
WebKit Commit Bot
Comment 17 2016-04-29 18:03:01 PDT
Attachment 277770 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/runtime/LazyProperty.h:103: The parameter name "initializer" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/runtime/LazyClassStructure.h:45: The parameter name "prototype" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/runtime/LazyClassStructure.h:59: The parameter name "constructor" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:344: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:348: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:353: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:357: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:367: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:371: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:377: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:385: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:389: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:403: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:410: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:429: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:437: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:442: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:446: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:451: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:456: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:463: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:467: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:473: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:477: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:577: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:582: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:586: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:591: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/LazyClassStructureInlines.h:38: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/heap/Heap.cpp:1132: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.h:34: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 31 in 34 files If any of these errors are false positives, please file a bug against check-webkit-style.
Filip Pizlo
Comment 18 2016-04-30 12:07:13 PDT
Created attachment 277820 [details] with more fixes
WebKit Commit Bot
Comment 19 2016-04-30 12:09:29 PDT
Attachment 277820 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/runtime/LazyProperty.h:103: The parameter name "initializer" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/runtime/LazyClassStructure.h:45: The parameter name "prototype" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/runtime/LazyClassStructure.h:59: The parameter name "constructor" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:344: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:348: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:353: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:357: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:367: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:371: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:377: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:385: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:389: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:403: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:410: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:429: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:437: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:442: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:446: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:451: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:456: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:463: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:467: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:473: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:477: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:577: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:582: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:586: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:591: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/LazyClassStructureInlines.h:38: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/heap/Heap.cpp:1132: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.h:34: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 31 in 34 files If any of these errors are false positives, please file a bug against check-webkit-style.
Filip Pizlo
Comment 20 2016-04-30 12:12:15 PDT
Created attachment 277821 [details] and now, even less broken!
WebKit Commit Bot
Comment 21 2016-04-30 12:14:24 PDT
Attachment 277821 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/runtime/LazyProperty.h:103: The parameter name "initializer" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/runtime/LazyClassStructure.h:45: The parameter name "prototype" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/runtime/LazyClassStructure.h:59: The parameter name "constructor" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:344: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:348: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:353: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:357: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:367: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:371: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:377: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:385: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:389: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:403: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:410: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:429: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:437: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:442: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:446: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:451: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:456: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:463: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:467: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:473: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:477: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:577: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:582: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:586: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:591: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/LazyClassStructureInlines.h:38: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/heap/Heap.cpp:1132: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.h:34: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 31 in 34 files If any of these errors are false positives, please file a bug against check-webkit-style.
Filip Pizlo
Comment 22 2016-04-30 12:23:04 PDT
Created attachment 277822 [details] less broken
WebKit Commit Bot
Comment 23 2016-04-30 12:25:36 PDT
Attachment 277822 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/runtime/LazyProperty.h:103: The parameter name "initializer" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/runtime/LazyClassStructure.h:45: The parameter name "prototype" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/runtime/LazyClassStructure.h:59: The parameter name "constructor" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:344: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:348: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:353: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:357: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:367: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:371: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:377: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:385: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:389: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:403: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:410: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:429: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:437: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:442: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:446: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:451: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:456: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:463: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:467: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:473: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:477: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:577: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:582: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:586: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:591: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/LazyClassStructureInlines.h:38: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/heap/Heap.cpp:1132: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.h:34: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 31 in 34 files If any of these errors are false positives, please file a bug against check-webkit-style.
Build Bot
Comment 24 2016-04-30 13:17:36 PDT
Comment on attachment 277822 [details] less broken Attachment 277822 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/1246343 Number of test failures exceeded the failure limit.
Build Bot
Comment 25 2016-04-30 13:17:40 PDT
Created attachment 277827 [details] Archive of layout-test-results from ews114 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews114 Port: mac-yosemite Platform: Mac OS X 10.10.5
Build Bot
Comment 26 2016-04-30 13:28:02 PDT
Comment on attachment 277822 [details] less broken Attachment 277822 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/1246366 Number of test failures exceeded the failure limit.
Build Bot
Comment 27 2016-04-30 13:28:07 PDT
Created attachment 277828 [details] Archive of layout-test-results from ews103 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews103 Port: mac-yosemite Platform: Mac OS X 10.10.5
Build Bot
Comment 28 2016-04-30 13:38:21 PDT
Comment on attachment 277822 [details] less broken Attachment 277822 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/1246382 Number of test failures exceeded the failure limit.
Build Bot
Comment 29 2016-04-30 13:38:25 PDT
Comment on attachment 277822 [details] less broken Attachment 277822 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/1246373 New failing tests: js/dom/global-constructors-attributes-idb.html jquery/core.html js/dom/global-constructors-attributes-dedicated-worker.html fast/dom/Window/window-properties-configurable.html
Build Bot
Comment 30 2016-04-30 13:38:26 PDT
Created attachment 277829 [details] Archive of layout-test-results from ews107 for mac-yosemite-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews107 Port: mac-yosemite-wk2 Platform: Mac OS X 10.10.5
Build Bot
Comment 31 2016-04-30 13:38:29 PDT
Created attachment 277830 [details] Archive of layout-test-results from ews126 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews126 Port: ios-simulator-wk2 Platform: Mac OS X 10.11.4
Filip Pizlo
Comment 32 2016-05-01 11:27:36 PDT
Comment on attachment 277822 [details] less broken I'm still fixing bugs revealed by WebCore and debug.
Filip Pizlo
Comment 33 2016-05-01 11:54:52 PDT
Created attachment 277866 [details] more Fixes a bunch of bugs where the lazy initialization of structures clashed with our allocator's isInitializing assertion. It's good that we have this assertion because this was a real bug. This is subtly wrong: JSBlah* blah = allocateCell<things>(stuff, globalObject->blahStructure(), more); In the case that blahStructure() is doing a LazyProperty<Structure>::get(), because by the time we call it, we are already in the middle of allocating. The heap may already contain a half-initialized JSBlah. If we GC'd inside LazyProperty::get(), we'd have a bad time. Fortunately, there were only a handful of places where we did this: - Some of them were accessing structures that shouldn't have been lazy, like nullGetterFunction and friends. I was wrong to make them lazy because it turns out that we would always trigger their initialization transitively from JSGlobalObject::init(). - The rest just needed to be changed to do this: Structure* structure = globalObject->blahStructure(); JSBlah* blah = allocateCell<things>(stuff, structure, more); There are only a few such places, so it's not so bad. Note that this is still crashing in debug for me, but less so. I'll leave it up to reviewers if they want to look at the patch in this current form. I suspect that there is still a long tail of small fixes like this one. That said, the patch now passes all tests in release for me (WebKit and JSC tests).
WebKit Commit Bot
Comment 34 2016-05-01 11:57:44 PDT
Attachment 277866 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/runtime/LazyProperty.h:103: The parameter name "initializer" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/runtime/LazyClassStructure.h:45: The parameter name "prototype" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/runtime/LazyClassStructure.h:59: The parameter name "constructor" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:344: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:348: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:353: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:357: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:367: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:371: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:377: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:397: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:404: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:423: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:431: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:436: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:440: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:445: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:450: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:457: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:461: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:467: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:471: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:571: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:576: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:580: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:585: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/LazyClassStructureInlines.h:38: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/heap/Heap.cpp:1134: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.h:34: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 29 in 37 files If any of these errors are false positives, please file a bug against check-webkit-style.
Build Bot
Comment 35 2016-05-01 12:52:29 PDT
Comment on attachment 277866 [details] more Attachment 277866 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/1251705 Number of test failures exceeded the failure limit.
Build Bot
Comment 36 2016-05-01 12:52:33 PDT
Created attachment 277867 [details] Archive of layout-test-results from ews113 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews113 Port: mac-yosemite Platform: Mac OS X 10.10.5
Filip Pizlo
Comment 37 2016-05-01 15:10:52 PDT
Created attachment 277871 [details] maybe it's done now
WebKit Commit Bot
Comment 38 2016-05-01 15:12:38 PDT
Attachment 277871 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/runtime/LazyProperty.h:103: The parameter name "initializer" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:344: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:348: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:353: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:357: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:367: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:371: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:377: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:397: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:404: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:423: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:431: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:436: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:440: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:445: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:450: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:457: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:461: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:467: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:471: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:571: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:576: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:580: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:585: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/LazyClassStructure.h:45: The parameter name "prototype" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/runtime/LazyClassStructure.h:59: The parameter name "constructor" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/runtime/LazyClassStructureInlines.h:38: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/heap/Heap.cpp:1134: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.h:34: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 29 in 41 files If any of these errors are false positives, please file a bug against check-webkit-style.
Filip Pizlo
Comment 39 2016-05-01 16:38:14 PDT
Created attachment 277873 [details] it's more done than ever before! I just found out that this makes SunSpider run 1% faster. Makes sense.
WebKit Commit Bot
Comment 40 2016-05-01 16:41:21 PDT
Attachment 277873 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/runtime/LazyProperty.h:103: The parameter name "initializer" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:344: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:348: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:353: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:357: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:367: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:371: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:377: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:397: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:404: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:423: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:431: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:436: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:440: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:445: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:450: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:457: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:461: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:467: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:471: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:571: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:576: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:580: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:585: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/LazyClassStructure.h:45: The parameter name "prototype" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/runtime/LazyClassStructure.h:59: The parameter name "constructor" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/runtime/LazyClassStructureInlines.h:38: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/heap/Heap.cpp:1134: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.h:34: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 29 in 42 files If any of these errors are false positives, please file a bug against check-webkit-style.
Filip Pizlo
Comment 41 2016-05-01 16:57:43 PDT
Created attachment 277875 [details] even more fixes Dealing with fallout from the change in JSWithScope API.
WebKit Commit Bot
Comment 42 2016-05-01 17:01:04 PDT
Attachment 277875 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/runtime/LazyProperty.h:103: The parameter name "initializer" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:344: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:348: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:353: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:357: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:367: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:371: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:377: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:397: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:404: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:423: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:431: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:436: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:440: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:445: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:450: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:457: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:461: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:467: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:471: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:571: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:576: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:580: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:585: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/LazyClassStructure.h:45: The parameter name "prototype" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/runtime/LazyClassStructure.h:59: The parameter name "constructor" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/runtime/LazyClassStructureInlines.h:38: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/heap/Heap.cpp:1134: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.h:34: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 29 in 43 files If any of these errors are false positives, please file a bug against check-webkit-style.
Filip Pizlo
Comment 43 2016-05-01 18:15:29 PDT
Created attachment 277882 [details] even more fixes
WebKit Commit Bot
Comment 44 2016-05-01 18:17:59 PDT
Attachment 277882 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/runtime/LazyProperty.h:103: The parameter name "initializer" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:344: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:348: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:353: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:357: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:367: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:371: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:377: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:397: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:404: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:423: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:431: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:436: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:440: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:445: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:450: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:457: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:461: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:467: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:471: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:571: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:576: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:580: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:585: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/LazyClassStructure.h:45: The parameter name "prototype" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/runtime/LazyClassStructure.h:59: The parameter name "constructor" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/runtime/LazyClassStructureInlines.h:38: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/heap/Heap.cpp:1134: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.h:34: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 29 in 43 files If any of these errors are false positives, please file a bug against check-webkit-style.
Filip Pizlo
Comment 45 2016-05-01 19:01:01 PDT
Created attachment 277885 [details] removed some Heap.cpp debugging hacks
WebKit Commit Bot
Comment 46 2016-05-01 19:03:27 PDT
Attachment 277885 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/runtime/LazyProperty.h:103: The parameter name "initializer" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:344: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:348: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:353: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:357: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:367: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:371: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:377: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:397: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:404: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:423: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:431: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:436: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:440: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:445: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:450: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:457: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:461: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:467: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:471: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:571: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:576: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:580: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:585: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/LazyClassStructure.h:45: The parameter name "prototype" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/runtime/LazyClassStructure.h:59: The parameter name "constructor" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/runtime/LazyClassStructureInlines.h:38: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.h:34: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 28 in 42 files If any of these errors are false positives, please file a bug against check-webkit-style.
Build Bot
Comment 47 2016-05-01 20:17:22 PDT
Comment on attachment 277885 [details] removed some Heap.cpp debugging hacks Attachment 277885 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/1253338 Number of test failures exceeded the failure limit.
Build Bot
Comment 48 2016-05-01 20:17:28 PDT
Created attachment 277887 [details] Archive of layout-test-results from ews116 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews116 Port: mac-yosemite Platform: Mac OS X 10.10.5
Filip Pizlo
Comment 49 2016-05-01 21:16:50 PDT
Created attachment 277888 [details] even better Fixes even more cases of the lazy structure getter being invoked during object initialization.
WebKit Commit Bot
Comment 50 2016-05-01 21:18:55 PDT
Attachment 277888 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/runtime/LazyProperty.h:103: The parameter name "initializer" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:344: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:348: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:353: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:357: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:367: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:371: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:377: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:397: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:404: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:423: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:431: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:436: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:440: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:445: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:450: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:457: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:461: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:467: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:471: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:571: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:576: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:580: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:585: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/LazyClassStructure.h:45: The parameter name "prototype" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/runtime/LazyClassStructure.h:59: The parameter name "constructor" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/runtime/LazyClassStructureInlines.h:38: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.h:34: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/JavaScriptCore/debugger/DebuggerScope.h:41: The parameter name "vm" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/debugger/DebuggerScope.h:41: The parameter name "scope" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 30 in 45 files If any of these errors are false positives, please file a bug against check-webkit-style.
Filip Pizlo
Comment 51 2016-05-02 10:19:57 PDT
Created attachment 277911 [details] even betterer Juggled the template instantiations around a bit to make LazyProperty<> nicer.
Filip Pizlo
Comment 52 2016-05-02 11:54:52 PDT
Created attachment 277918 [details] rebased Wow, rebasing this patch is so annoying.
WebKit Commit Bot
Comment 53 2016-05-02 11:56:37 PDT
Attachment 277918 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/runtime/LazyProperty.h:94: The parameter name "owner" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/runtime/LazyProperty.h:95: The parameter name "owner" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:344: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:348: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:353: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:357: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:367: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:371: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:377: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:385: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:405: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:412: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:431: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:439: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:444: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:448: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:453: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:458: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:465: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:469: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:475: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:479: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:579: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:584: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:588: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:593: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/LazyClassStructure.h:45: The parameter name "prototype" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/runtime/LazyClassStructure.h:59: The parameter name "constructor" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/runtime/LazyClassStructureInlines.h:38: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.h:34: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/JavaScriptCore/debugger/DebuggerScope.h:41: The parameter name "vm" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/debugger/DebuggerScope.h:41: The parameter name "scope" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 32 in 45 files If any of these errors are false positives, please file a bug against check-webkit-style.
Keith Miller
Comment 54 2016-05-02 16:19:48 PDT
Comment on attachment 277918 [details] rebased View in context: https://bugs.webkit.org/attachment.cgi?id=277918&action=review r=me > Source/JavaScriptCore/ChangeLog:96 > + this property, and when it needs to be initialized, Loolup will assume you have a Typo: Loolup => Lookup
Filip Pizlo
Comment 55 2016-05-02 16:23:09 PDT
(In reply to comment #54) > Comment on attachment 277918 [details] > rebased > > View in context: > https://bugs.webkit.org/attachment.cgi?id=277918&action=review > > r=me > > > Source/JavaScriptCore/ChangeLog:96 > > + this property, and when it needs to be initialized, Loolup will assume you have a > > Typo: Loolup => Lookup Fixed!
Saam Barati
Comment 56 2016-05-02 21:39:18 PDT
Is this a JSBench speed up?
Filip Pizlo
Comment 57 2016-05-03 11:12:20 PDT
(In reply to comment #56) > Is this a JSBench speed up? Haven't measured yet. I think I'll let the bots do that for me.
Filip Pizlo
Comment 58 2016-05-03 11:36:55 PDT
Filip Pizlo
Comment 59 2016-05-03 12:04:44 PDT
(In reply to comment #57) > (In reply to comment #56) > > Is this a JSBench speed up? > > Haven't measured yet. I think I'll let the bots do that for me. BTW, now that I think about it, this may not affect in-browser performance if the DOM's subclasses of JSGlobalObject do super expensive things. I don't know if that's the case or not. I'll watch the bots.
Csaba Osztrogonác
Comment 60 2016-05-04 03:00:32 PDT
(In reply to comment #58) > Landed in http://trac.webkit.org/changeset/200383 It made all tests crash on ARMv7 Thumb2, see bug157340 for details.
Csaba Osztrogonác
Comment 61 2016-05-04 04:02:19 PDT
Comment on attachment 277918 [details] rebased View in context: https://bugs.webkit.org/attachment.cgi?id=277918&action=review > Source/JavaScriptCore/runtime/LazyProperty.h:81 > + auto callFunc = bitwise_cast<ElementType* (*)(const Initializer&)>(m_pointer & ~lazyTag); > + return callFunc(Initializer(const_cast<OwnerType*>(owner), *const_cast<LazyProperty*>(this))); Removing the least significant bit is completely incorrect on ARMv7 Thumb2. The 1 bit means for BLX remaining in Thumb2 mode, but 0 bit makes the CPU switch to ARM mode, but there are Thumb2 instructions on that address.
Chris Dumez
Comment 62 2016-05-04 08:36:07 PDT
FUI, we started to experience crashes on iOS PLT since around this patch landed. I haven't confirmed yet the crashes are caused by this change but it is the most likely culprit.
Chris Dumez
Comment 63 2016-05-04 08:41:19 PDT
(In reply to comment #62) > FUI, we started to experience crashes on iOS PLT since around this patch > landed. I haven't confirmed yet the crashes are caused by this change but it > is the most likely culprit. I see you landed a follow-up crash fix. However, we still see the crashes on iOS on r200415.
Chris Dumez
Comment 64 2016-05-04 08:47:41 PDT
Reverted r200383 and r200406 for reason: Seems to have caused crashes on iOS / ARMv7s Committed r200416: <http://trac.webkit.org/changeset/200416>
Chris Dumez
Comment 65 2016-05-04 08:49:33 PDT
(In reply to comment #64) > Reverted r200383 and r200406 for reason: > > Seems to have caused crashes on iOS / ARMv7s > > Committed r200416: <http://trac.webkit.org/changeset/200416> I am working on getting you a symbolicated stack trace.
Chris Dumez
Comment 66 2016-05-04 08:58:02 PDT
(In reply to comment #65) > (In reply to comment #64) > > Reverted r200383 and r200406 for reason: > > > > Seems to have caused crashes on iOS / ARMv7s > > > > Committed r200416: <http://trac.webkit.org/changeset/200416> > > I am working on getting you a symbolicated stack trace. I sent you the full trace by mail but here is the short version: Exception Type: EXC_BAD_INSTRUCTION (SIGILL) Exception Codes: 0x0000000000000001, 0x00000000f012680a Triggered by Thread: 0 Thread 0 name: Dispatch queue: com.apple.main-thread Thread 0 Crashed: 0 JavaScriptCore 0x00fd8270 JSC::JSFunction* JSC::LazyProperty<JSC::JSGlobalObject, JSC::JSFunction>::callFunc<JSC::JSGlobalObject::init(JSC::VM&)::$_4>(JSC::LazyProperty<JSC::JSGlobalObject, JSC::JSFunction>::Initializer const&) + 16 1 JavaScriptCore 0x00d38104 JSC::ArrayPrototype::finishCreation(JSC::VM&, JSC::JSGlobalObject*) + 4576 2 JavaScriptCore 0x00d36efe JSC::ArrayPrototype::create(JSC::VM&, JSC::JSGlobalObject*, JSC::Structure*) + 78 3 JavaScriptCore 0x00fd2544 JSC::JSGlobalObject::init(JSC::VM&) + 3580 4 WebCore 0x01b984f8 WebCore::JSDOMGlobalObject::finishCreation(JSC::VM&, JSC::JSObject*) + 60 5 WebCore 0x01be553c WebCore::JSDOMWindowBase::finishCreation(JSC::VM&, WebCore::JSDOMWindowShell*) + 40 6 WebCore 0x01be1196 WebCore::JSDOMWindow::finishCreation(JSC::VM&, WebCore::JSDOMWindowShell*) + 14 7 WebCore 0x01bf2b32 WebCore::JSDOMWindowShell::setWindow(WTF::PassRefPtr<WebCore::DOMWindow>) + 450 8 WebCore 0x01bf2948 WebCore::JSDOMWindowShell::finishCreation(JSC::VM&, WTF::PassRefPtr<WebCore::DOMWindow>) + 20 9 WebCore 0x0206b1cc WebCore::ScriptController::createWindowShell(WebCore::DOMWrapperWorld&) + 172 10 WebCore 0x0206ba08 WebCore::ScriptController::initScript(WebCore::DOMWrapperWorld&) + 32 11 WebKit 0x0029cf84 WebKit::WebFrame::jsContextForWorld(WebKit::InjectedBundleScriptWorld*) + 24
Zan Dobersek
Comment 67 2016-05-04 12:03:02 PDT
Regressions on non-optimized builds with GCC, as explained in bug #157338, are due to callFunc instantiations not being aligned. Thumb2 failures are due to the lazy tag conflicting with the arch-specific use of the bottom bit. Force-aligning callFunc() to 16-byte boundaries and shifting the tags to the second and third lowest bits fixes the issues.
Filip Pizlo
Comment 68 2016-05-04 12:31:12 PDT
(In reply to comment #67) > Regressions on non-optimized builds with GCC, as explained in bug #157338, > are due to callFunc instantiations not being aligned. Thumb2 failures are > due to the lazy tag conflicting with the arch-specific use of the bottom bit. > > Force-aligning callFunc() to 16-byte boundaries and shifting the tags to the > second and third lowest bits fixes the issues. Yup. It was a mistake for me to use pointer tagging on function pointers. I have an idea of how to fix that. Working on it now.
Filip Pizlo
Comment 69 2016-05-04 12:51:21 PDT
(In reply to comment #68) > (In reply to comment #67) > > Regressions on non-optimized builds with GCC, as explained in bug #157338, > > are due to callFunc instantiations not being aligned. Thumb2 failures are > > due to the lazy tag conflicting with the arch-specific use of the bottom bit. > > > > Force-aligning callFunc() to 16-byte boundaries and shifting the tags to the > > second and third lowest bits fixes the issues. > > Yup. It was a mistake for me to use pointer tagging on function pointers. > I have an idea of how to fix that. Working on it now. I think I have a glorious fix. LazyProperty<>::m_pointer will now point to a global variable that points to the function. That way, we can do pointer tagging in m_pointer because a pointer to a global pointer variable is guaranteed to be at least 4-bytes aligned and it will not have anything like the Thumb2 tag in it. Testing this now...
Filip Pizlo
Comment 70 2016-05-04 13:02:40 PDT
*** Bug 157333 has been marked as a duplicate of this bug. ***
Filip Pizlo
Comment 71 2016-05-04 13:02:49 PDT
*** Bug 157338 has been marked as a duplicate of this bug. ***
Filip Pizlo
Comment 72 2016-05-04 13:04:10 PDT
*** Bug 157340 has been marked as a duplicate of this bug. ***
Filip Pizlo
Comment 73 2016-05-04 13:08:26 PDT
Created attachment 278115 [details] patch for relanding
Filip Pizlo
Comment 74 2016-05-04 13:31:36 PDT
Created attachment 278121 [details] patch for relanding
WebKit Commit Bot
Comment 75 2016-05-04 13:33:51 PDT
Attachment 278121 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/runtime/LazyProperty.h:98: The parameter name "owner" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/runtime/LazyProperty.h:99: The parameter name "owner" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:343: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:347: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:352: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:356: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:366: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:370: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:376: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:384: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:405: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:412: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:431: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:439: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:444: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:448: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:453: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:458: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:465: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:469: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:475: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:479: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:579: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:584: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:588: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:593: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/LazyClassStructure.h:45: The parameter name "prototype" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/runtime/LazyClassStructure.h:59: The parameter name "constructor" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/runtime/LazyClassStructureInlines.h:38: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.h:34: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/JavaScriptCore/debugger/DebuggerScope.h:41: The parameter name "vm" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/debugger/DebuggerScope.h:41: The parameter name "scope" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 32 in 48 files If any of these errors are false positives, please file a bug against check-webkit-style.
Filip Pizlo
Comment 76 2016-05-04 14:21:36 PDT
Note You need to log in before you can comment on or make changes to this bug.