Bug 157045

Summary: Speed up JSGlobalObject initialization by making some properties lazy
Product: WebKit Reporter: Filip Pizlo <fpizlo>
Component: JavaScriptCoreAssignee: Filip Pizlo <fpizlo>
Status: RESOLVED FIXED    
Severity: Normal CC: barraclough, benjamin, buildbot, cdumez, cgarcia, commit-queue, ggaren, joepeck, keith_miller, mark.lam, mhahnenb, msaboff, oliver, ossy, rniwa, sam, sbarati, zan
Priority: P2    
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Bug Depends on: 157333, 157338, 157340    
Bug Blocks:    
Attachments:
Description Flags
work in progress
none
a bit more
none
some more
none
it's getting weird
none
moar
none
getting close
none
it compiles a bit more
none
OMG JSGlobalObject.cpp compiles
none
2x faster to create JSGlobalObject
none
less wrong
none
totally awesome
none
rebased
none
with fixes
none
with more fixes
none
and now, even less broken!
none
less broken
none
Archive of layout-test-results from ews114 for mac-yosemite
none
Archive of layout-test-results from ews103 for mac-yosemite
none
Archive of layout-test-results from ews107 for mac-yosemite-wk2
none
Archive of layout-test-results from ews126 for ios-simulator-wk2
none
more
buildbot: commit-queue-
Archive of layout-test-results from ews113 for mac-yosemite
none
maybe it's done now
none
it's more done than ever before!
none
even more fixes
none
even more fixes
none
removed some Heap.cpp debugging hacks
buildbot: commit-queue-
Archive of layout-test-results from ews116 for mac-yosemite
none
even better
none
even betterer
none
rebased
keith_miller: review+
patch for relanding
none
patch for relanding none

Description Filip Pizlo 2016-04-26 14:54:11 PDT
Patch forthcoming.
Comment 1 Filip Pizlo 2016-04-26 14:54:56 PDT
Created attachment 277410 [details]
work in progress
Comment 2 Filip Pizlo 2016-04-26 17:03:48 PDT
Created attachment 277424 [details]
a bit more

I'm starting to have a plan.
Comment 3 Filip Pizlo 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.
Comment 4 Filip Pizlo 2016-04-26 21:30:48 PDT
Created attachment 277440 [details]
some more
Comment 5 Filip Pizlo 2016-04-27 14:55:08 PDT
Created attachment 277539 [details]
it's getting weird

But it will be even weirder soon.
Comment 6 Filip Pizlo 2016-04-27 20:50:42 PDT
Created attachment 277580 [details]
moar
Comment 7 Filip Pizlo 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.
Comment 8 Filip Pizlo 2016-04-28 12:31:49 PDT
Created attachment 277640 [details]
it compiles a bit more

Still lots of compiler errors though
Comment 9 Filip Pizlo 2016-04-28 14:25:48 PDT
Created attachment 277649 [details]
OMG JSGlobalObject.cpp compiles

Wow
Comment 10 Filip Pizlo 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.
Comment 11 Filip Pizlo 2016-04-28 15:19:26 PDT
*** Bug 157044 has been marked as a duplicate of this bug. ***
Comment 12 Filip Pizlo 2016-04-28 15:46:18 PDT
Created attachment 277656 [details]
less wrong

Still dealing with basics.
Comment 13 Filip Pizlo 2016-04-29 16:18:54 PDT
Created attachment 277752 [details]
totally awesome

It passes tests!
Comment 14 Filip Pizlo 2016-04-29 17:13:29 PDT
Created attachment 277762 [details]
rebased
Comment 15 WebKit Commit Bot 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.
Comment 16 Filip Pizlo 2016-04-29 18:01:15 PDT
Created attachment 277770 [details]
with fixes
Comment 17 WebKit Commit Bot 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.
Comment 18 Filip Pizlo 2016-04-30 12:07:13 PDT
Created attachment 277820 [details]
with more fixes
Comment 19 WebKit Commit Bot 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.
Comment 20 Filip Pizlo 2016-04-30 12:12:15 PDT
Created attachment 277821 [details]
and now, even less broken!
Comment 21 WebKit Commit Bot 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.
Comment 22 Filip Pizlo 2016-04-30 12:23:04 PDT
Created attachment 277822 [details]
less broken
Comment 23 WebKit Commit Bot 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.
Comment 24 Build Bot 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.
Comment 25 Build Bot 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
Comment 26 Build Bot 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.
Comment 27 Build Bot 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
Comment 28 Build Bot 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.
Comment 29 Build Bot 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
Comment 30 Build Bot 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
Comment 31 Build Bot 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
Comment 32 Filip Pizlo 2016-05-01 11:27:36 PDT
Comment on attachment 277822 [details]
less broken

I'm still fixing bugs revealed by WebCore and debug.
Comment 33 Filip Pizlo 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).
Comment 34 WebKit Commit Bot 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.
Comment 35 Build Bot 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.
Comment 36 Build Bot 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
Comment 37 Filip Pizlo 2016-05-01 15:10:52 PDT
Created attachment 277871 [details]
maybe it's done now
Comment 38 WebKit Commit Bot 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.
Comment 39 Filip Pizlo 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.
Comment 40 WebKit Commit Bot 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.
Comment 41 Filip Pizlo 2016-05-01 16:57:43 PDT
Created attachment 277875 [details]
even more fixes

Dealing with fallout from the change in JSWithScope API.
Comment 42 WebKit Commit Bot 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.
Comment 43 Filip Pizlo 2016-05-01 18:15:29 PDT
Created attachment 277882 [details]
even more fixes
Comment 44 WebKit Commit Bot 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.
Comment 45 Filip Pizlo 2016-05-01 19:01:01 PDT
Created attachment 277885 [details]
removed some Heap.cpp debugging hacks
Comment 46 WebKit Commit Bot 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.
Comment 47 Build Bot 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.
Comment 48 Build Bot 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
Comment 49 Filip Pizlo 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.
Comment 50 WebKit Commit Bot 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.
Comment 51 Filip Pizlo 2016-05-02 10:19:57 PDT
Created attachment 277911 [details]
even betterer

Juggled the template instantiations around a bit to make LazyProperty<> nicer.
Comment 52 Filip Pizlo 2016-05-02 11:54:52 PDT
Created attachment 277918 [details]
rebased

Wow, rebasing this patch is so annoying.
Comment 53 WebKit Commit Bot 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.
Comment 54 Keith Miller 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
Comment 55 Filip Pizlo 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!
Comment 56 Saam Barati 2016-05-02 21:39:18 PDT
Is this a JSBench speed up?
Comment 57 Filip Pizlo 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.
Comment 58 Filip Pizlo 2016-05-03 11:36:55 PDT
Landed in http://trac.webkit.org/changeset/200383
Comment 59 Filip Pizlo 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.
Comment 60 Csaba Osztrogon√°c 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.
Comment 61 Csaba Osztrogon√°c 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.
Comment 62 Chris Dumez 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.
Comment 63 Chris Dumez 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.
Comment 64 Chris Dumez 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>
Comment 65 Chris Dumez 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.
Comment 66 Chris Dumez 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
Comment 67 Zan Dobersek 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.
Comment 68 Filip Pizlo 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.
Comment 69 Filip Pizlo 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...
Comment 70 Filip Pizlo 2016-05-04 13:02:40 PDT
*** Bug 157333 has been marked as a duplicate of this bug. ***
Comment 71 Filip Pizlo 2016-05-04 13:02:49 PDT
*** Bug 157338 has been marked as a duplicate of this bug. ***
Comment 72 Filip Pizlo 2016-05-04 13:04:10 PDT
*** Bug 157340 has been marked as a duplicate of this bug. ***
Comment 73 Filip Pizlo 2016-05-04 13:08:26 PDT
Created attachment 278115 [details]
patch for relanding
Comment 74 Filip Pizlo 2016-05-04 13:31:36 PDT
Created attachment 278121 [details]
patch for relanding
Comment 75 WebKit Commit Bot 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.
Comment 76 Filip Pizlo 2016-05-04 14:21:36 PDT
Landed in http://trac.webkit.org/changeset/200430