Bug 90347 – Inline property storage should not be wasted when it is exhausted

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Filip Pizlo]

Currently JSFinalObjects (i.e. vanilla JS objects created via 'new Object', '{}', or constructor calls to user-defined functions) will have inline property storage, enough for at least 4 properties.  This is a great optimization since it means that for most objects we don't need to perform a second allocation to create property storage, and accesses to properties of small objects are fast.

But things go wrong when the inline property storage is exhausted: we allocate out-of-line storage and move all properties that were inline into the out-of-line storage.  This simplifies property accesses, as it implies that it's always safe to say object->propertyStorage[offset], where object->propertyStorage points to either the inline storage or the out-of-line storage depending on the object size.  But this simplicity comes with three costs: first, wasted memory since the inline storage slots are not reclaimed; and second, execution time penalty when the first out-of-line property is stored since all of the inline properties have to be copied to the out-of-line storage; and third, increased execution time cost for accessing the properties that were previously inline.

A better approach would be to continue to use the inline property storage even when it is exhausted: the first N properties will always be inline and the remainder will be out-of-line.  Since our inline property storage size is tuned for common object sizes, this means that the vast majority of properties in the heap will tend to be inline and will be fast to access.  This will be a performance improvement over the current approach, where as soon as any property overflows the inline storage, all properties are evicted and become more expensive to access.

[Filip Pizlo]

Created attachment 150328 [details]
it begins

Still a lot more to do.  So far I've written probably most of the glue necessary for the 32-bit LLInt to work, and have modified most of the runtime.  But I still need to find all of the remaining places where the offset logic needs changing, and I still need to tweak the other execution engines.  That will probably be a lot of work, and there's no choice but to do it in one go.

[Filip Pizlo]

Created attachment 150335 [details]
more!

[Filip Pizlo]

Created attachment 150339 [details]
it grows!

[Filip Pizlo]

Created attachment 150340 [details]
the JIT has been attacked

But I still need to do the horrific load-to-nop patching code.  And I still need to write the same code for the old JIT.

[Filip Pizlo]

Created attachment 150346 [details]
prop storage load patching is implemented

The X86 version of the code to flip the out-of-line storage load between being active and inactive is written.  I decided to go for the load-to-lea trick that Gavin suggested.

[Filip Pizlo]

Created attachment 150351 [details]
a lot more code

Property name iteration ... it's special.

[Filip Pizlo]

Created attachment 150377 [details]
property name iteration done right

The glue for 32-bit is almost done!  Oddly, I'm doing 32-bit first this time; I'll do 64-bit after 32-bit starts working.

[Filip Pizlo]

Zoltan, I've CC'd you because this will require new assembler support: we'll need to be able to turn a

loadPtr(Address(register1, constant), register2)

into a

addPtr(constant, register1, register2)

and vice-versa in RepatchBuffer.  This patch doesn't build yet and is still in the relatively early stages.  I hope to have it finished in a day or so.

[Filip Pizlo]

Created attachment 150530 [details]
wrote some 64-bit thingies

It still probably doesn't compile.

[Filip Pizlo]

Created attachment 150535 [details]
fixing some compiler errors

But of course it doesn't compile yet!

[Filip Pizlo]

Created attachment 150540 [details]
it compiles!

In 64-bit mode anyway.

[Filip Pizlo]

Created attachment 150546 [details]
I proved it correct

by running "hello world".

[Filip Pizlo]

Created attachment 150680 [details]
it might work

Still doing some more testing though.

[WebKit Review Bot]

Attachment 150680 [details] did not pass style-queue:

Source/JavaScriptCore/runtime/PropertyMapHashTable.h:179:  The parameter name "offset" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/PropertyMapHashTable.h:181:  The parameter name "type" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:375:  The parameter name "returnAddress" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:376:  The parameter name "returnAddress" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:394:  The parameter name "returnAddress" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:394:  The parameter name "callFrame" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:396:  The parameter name "callFrame" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:397:  The parameter name "chain" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:397:  The parameter name "callFrame" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:398:  The parameter name "returnAddress" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:398:  The parameter name "callFrame" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:399:  The parameter name "returnAddress" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/Structure.h:90:  The parameter name "offset" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/Structure.h:91:  The parameter name "offset" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/Structure.h:92:  The parameter name "offset" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/PutPropertySlot.h:67:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/JavaScriptCore/runtime/Options.cpp:137:  Line contains only semicolon. If this should be an empty statement, use { } instead.  [whitespace/semicolon] [5]
Source/JavaScriptCore/runtime/PropertyOffset.h:49:  The parameter name "offset" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/PropertyOffset.h:50:  The parameter name "offset" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/PropertyOffset.h:50:  The parameter name "type" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/PropertyOffset.h:51:  The parameter name "offset" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/PropertyOffset.h:52:  The parameter name "offset" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/PropertyOffset.h:52:  The parameter name "type" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/PropertyOffset.h:53:  The parameter name "offset" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/PropertyOffset.h:54:  The parameter name "offset" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/PropertyOffset.h:55:  The parameter name "offset" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/PropertyOffset.h:56:  The parameteFailed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1
r name "offset" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/PropertyOffset.h:57:  The parameter name "offset" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/PropertyOffset.h:58:  The parameter name "offset" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/PropertyOffset.h:59:  The parameter name "offset" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/PropertyOffset.h:60:  The parameter name "offset" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/PropertyOffset.h:60:  The parameter name "type" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/PropertyOffset.h:61:  The parameter name "offset" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/PropertyOffset.h:61:  The parameter name "type" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/PropertyOffset.h:62:  The parameter name "type" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:102:  The parameter name "offset" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 36 in 50 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Gyuyoung Kim]

Comment on attachment 150680 [details]
it might work

Attachment 150680 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/13124661

[Gustavo Noronha (kov)]

Comment on attachment 150680 [details]
it might work

Attachment 150680 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/13139304

[Build Bot]

Comment on attachment 150680 [details]
it might work

Attachment 150680 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/13127629

[Early Warning System Bot]

Comment on attachment 150680 [details]
it might work

Attachment 150680 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/13126593

[Early Warning System Bot]

Comment on attachment 150680 [details]
it might work

Attachment 150680 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/13131598

[Build Bot]

Comment on attachment 150680 [details]
it might work

Attachment 150680 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/13133548

[Filip Pizlo]

Created attachment 150701 [details]
it works on x86-32, x86-64, and classic interpreter

Still working on other backends.

[WebKit Review Bot]

Attachment 150701 [details] did not pass style-queue:

Source/JavaScriptCore/runtime/PropertyMapHashTable.h:179:  The parameter name "offset" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/PropertyMapHashTable.h:181:  The parameter name "type" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:375:  The parameter name "returnAddress" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:376:  The parameter name "returnAddress" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:394:  The parameter name "returnAddress" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:394:  The parameter name "callFrame" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:396:  The parameter name "callFrame" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:397:  The parameter name "chain" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:397:  The parameter name "callFrame" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:398:  The parameter name "returnAddress" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:398:  The parameter name "callFrame" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:399:  The parameter name "returnAddress" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/Structure.h:90:  The parameter name "offset" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/Structure.h:91:  The parameter name "offset" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/Structure.h:92:  The parameter name "offset" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/PutPropertySlot.h:67:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/JavaScriptCore/runtime/Options.cpp:137:  Line contains only semicolon. If this should be an empty statement, use { } instead.  [whitespace/semicolon] [5]
Source/JavaScriptCore/runtime/PropertyOffset.h:49:  The parameter name "offset" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/PropertyOffset.h:50:  The parameter name "offset" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/PropertyOffset.h:50:  The parameter name "type" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/PropertyOffset.h:51:  The parameter name "offset" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/PropertyOffset.h:52:  The parameter name "offset" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/PropertyOffset.h:52:  The parameter name "type" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/PropertyOffset.h:53:  The parameter name "offset" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/PropertyOffset.h:54:  The parameter name "offset" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/PropertyOffset.h:55:  The parameter name "offset" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/PropertyOffset.h:56:  The parameteFailed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1
r name "offset" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/PropertyOffset.h:57:  The parameter name "offset" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/PropertyOffset.h:58:  The parameter name "offset" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/PropertyOffset.h:59:  The parameter name "offset" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/PropertyOffset.h:60:  The parameter name "offset" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/PropertyOffset.h:60:  The parameter name "type" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/PropertyOffset.h:61:  The parameter name "offset" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/PropertyOffset.h:61:  The parameter name "type" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/PropertyOffset.h:62:  The parameter name "type" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:102:  The parameter name "offset" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 36 in 56 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Build Bot]

Comment on attachment 150701 [details]
it works on x86-32, x86-64, and classic interpreter

Attachment 150701 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/13125663

[Build Bot]

Comment on attachment 150701 [details]
it works on x86-32, x86-64, and classic interpreter

Attachment 150701 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/13133579

[Filip Pizlo]

Created attachment 150729 [details]
the patch

[WebKit Review Bot]

Attachment 150729 [details] did not pass style-queue:

Source/JavaScriptCore/runtime/PropertyMapHashTable.h:179:  The parameter name "offset" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/PropertyMapHashTable.h:181:  The parameter name "type" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:375:  The parameter name "returnAddress" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:376:  The parameter name "returnAddress" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:394:  The parameter name "returnAddress" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:394:  The parameter name "callFrame" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:396:  The parameter name "callFrame" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:397:  The parameter name "chain" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:397:  The parameter name "callFrame" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:398:  The parameter name "returnAddress" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:398:  The parameter name "callFrame" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:399:  The parameter name "returnAddress" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/Structure.h:90:  The parameter name "offset" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/Structure.h:91:  The parameter name "offset" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/Structure.h:92:  The parameter name "offset" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/PutPropertySlot.h:67:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/JavaScriptCore/runtime/Options.cpp:137:  Line contains only semicolon. If this should be an empty statement, use { } instead.  [whitespace/semicolon] [5]
Source/JavaScriptCore/runtime/PropertyOffset.h:49:  The parameter name "offset" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/PropertyOffset.h:50:  The parameter name "offset" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/PropertyOffset.h:50:  The parameter name "type" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/PropertyOffset.h:51:  The parameter name "offset" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/PropertyOffset.h:52:  The parameter name "offset" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/PropertyOffset.h:52:  The parameter name "type" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/PropertyOffset.h:53:  The parameter name "offset" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/PropertyOffset.h:54:  The parameter name "offset" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/PropertyOffset.h:55:  The parameter name "offset" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/PropertyOffset.h:56:  The parameteFailed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1
r name "offset" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/PropertyOffset.h:57:  The parameter name "offset" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/PropertyOffset.h:58:  The parameter name "offset" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/PropertyOffset.h:59:  The parameter name "offset" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/PropertyOffset.h:60:  The parameter name "offset" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/PropertyOffset.h:60:  The parameter name "type" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/PropertyOffset.h:61:  The parameter name "offset" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/PropertyOffset.h:61:  The parameter name "type" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/PropertyOffset.h:62:  The parameter name "type" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:102:  The parameter name "offset" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 36 in 60 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Filip Pizlo]

Created attachment 150732 [details]
the patch

[WebKit Review Bot]

Attachment 150732 [details] did not pass style-queue:

Source/JavaScriptCore/runtime/PropertyMapHashTable.h:179:  The parameter name "offset" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/PropertyMapHashTable.h:181:  The parameter name "type" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:375:  The parameter name "returnAddress" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:376:  The parameter name "returnAddress" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:394:  The parameter name "returnAddress" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:394:  The parameter name "callFrame" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:396:  The parameter name "callFrame" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:397:  The parameter name "chain" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:397:  The parameter name "callFrame" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:398:  The parameter name "returnAddress" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:398:  The parameter name "callFrame" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:399:  The parameter name "returnAddress" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/Structure.h:90:  The parameter name "offset" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/Structure.h:91:  The parameter name "offset" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/Structure.h:92:  The parameter name "offset" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/PutPropertySlot.h:67:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/JavaScriptCore/runtime/Options.cpp:137:  Line contains only semicolon. If this should be an empty statement, use { } instead.  [whitespace/semicolon] [5]
Source/JavaScriptCore/runtime/PropertyOffset.h:49:  The parameter name "offset" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/PropertyOffset.h:50:  The parameter name "offset" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/PropertyOffset.h:50:  The parameter name "type" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/PropertyOffset.h:51:  The parameter name "offset" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/PropertyOffset.h:52:  The parameter name "offset" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/PropertyOffset.h:52:  The parameter name "type" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/PropertyOffset.h:53:  The parameter name "offset" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/PropertyOffset.h:54:  The parameter name "offset" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/PropertyOffset.h:55:  The parameter name "offset" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/PropertyOffset.h:56:  The parameteFailed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1
r name "offset" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/PropertyOffset.h:57:  The parameter name "offset" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/PropertyOffset.h:58:  The parameter name "offset" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/PropertyOffset.h:59:  The parameter name "offset" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/PropertyOffset.h:60:  The parameter name "offset" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/PropertyOffset.h:60:  The parameter name "type" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/PropertyOffset.h:61:  The parameter name "offset" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/PropertyOffset.h:61:  The parameter name "type" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/PropertyOffset.h:62:  The parameter name "type" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:102:  The parameter name "offset" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 36 in 59 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Filip Pizlo]

Created attachment 150736 [details]
the patch

Fix style issues.

[WebKit Review Bot]

Attachment 150736 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1
Source/JavaScriptCore/jit/JIT.h:375:  The parameter name "returnAddress" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:376:  The parameter name "returnAddress" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:394:  The parameter name "returnAddress" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:394:  The parameter name "callFrame" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:396:  The parameter name "callFrame" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:397:  The parameter name "chain" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:397:  The parameter name "callFrame" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:398:  The parameter name "returnAddress" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:398:  The parameter name "callFrame" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:399:  The parameter name "returnAddress" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/Options.cpp:137:  Line contains only semicolon. If this should be an empty statement, use { } instead.  [whitespace/semicolon] [5]
Total errors found: 11 in 59 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Filip Pizlo]

Zoltan: I've finalized the approach of repatching loadPtr().  There's now a convertibleLoadPtr(), which can be converted into an address computation.  It should be a straight-forward API to implement, though it is necessarily gross.

[Build Bot]

Comment on attachment 150736 [details]
the patch

Attachment 150736 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/13133682

[Build Bot]

Comment on attachment 150736 [details]
the patch

Attachment 150736 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/13127786

[Filip Pizlo]

Created attachment 150835 [details]
the patch

Fixing build errors.

[Filip Pizlo]

Created attachment 150836 [details]
the patch

Just rebasing.

[WebKit Review Bot]

Attachment 150836 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1
Source/JavaScriptCore/jit/JIT.h:375:  The parameter name "returnAddress" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:376:  The parameter name "returnAddress" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:394:  The parameter name "returnAddress" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:394:  The parameter name "callFrame" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:396:  The parameter name "callFrame" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:397:  The parameter name "chain" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:397:  The parameter name "callFrame" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:398:  The parameter name "returnAddress" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:398:  The parameter name "callFrame" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:399:  The parameter name "returnAddress" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/Options.cpp:137:  Line contains only semicolon. If this should be an empty statement, use { } instead.  [whitespace/semicolon] [5]
Total errors found: 11 in 60 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Build Bot]

Comment on attachment 150836 [details]
the patch

Attachment 150836 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/13137798

[Build Bot]

Comment on attachment 150836 [details]
the patch

Attachment 150836 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/13125906

[Filip Pizlo]

Created attachment 150841 [details]
the patch

More build fixes.

[WebKit Review Bot]

Attachment 150841 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1
Source/JavaScriptCore/jit/JIT.h:375:  The parameter name "returnAddress" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:376:  The parameter name "returnAddress" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:394:  The parameter name "returnAddress" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:394:  The parameter name "callFrame" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:396:  The parameter name "callFrame" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:397:  The parameter name "chain" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:397:  The parameter name "callFrame" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:398:  The parameter name "returnAddress" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:398:  The parameter name "callFrame" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:399:  The parameter name "returnAddress" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/Options.cpp:138:  Line contains only semicolon. If this should be an empty statement, use { } instead.  [whitespace/semicolon] [5]
Total errors found: 11 in 60 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Build Bot]

Comment on attachment 150841 [details]
the patch

Attachment 150841 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/13125923

[Filip Pizlo]

Created attachment 150843 [details]
the patch

Even more build fixes.

[WebKit Review Bot]

Attachment 150843 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1
Source/JavaScriptCore/jit/JIT.h:375:  The parameter name "returnAddress" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:376:  The parameter name "returnAddress" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:394:  The parameter name "returnAddress" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:394:  The parameter name "callFrame" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:396:  The parameter name "callFrame" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:397:  The parameter name "chain" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:397:  The parameter name "callFrame" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:398:  The parameter name "returnAddress" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:398:  The parameter name "callFrame" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:399:  The parameter name "returnAddress" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/Options.cpp:138:  Line contains only semicolon. If this should be an empty statement, use { } instead.  [whitespace/semicolon] [5]
Total errors found: 11 in 61 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Filip Pizlo]

Created attachment 150851 [details]
the patch

More build fixes.  Apparently, the compiler the Mac build bot uses has a whole bunch of pedantry I've magically managed to avoid on my install.

[WebKit Review Bot]

Attachment 150851 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1
Source/JavaScriptCore/jit/JIT.h:375:  The parameter name "returnAddress" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:376:  The parameter name "returnAddress" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:394:  The parameter name "returnAddress" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:394:  The parameter name "callFrame" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:396:  The parameter name "callFrame" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:397:  The parameter name "chain" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:397:  The parameter name "callFrame" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:398:  The parameter name "returnAddress" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:398:  The parameter name "callFrame" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:399:  The parameter name "returnAddress" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/Options.cpp:138:  Line contains only semicolon. If this should be an empty statement, use { } instead.  [whitespace/semicolon] [5]
Total errors found: 11 in 61 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Build Bot]

Comment on attachment 150851 [details]
the patch

Attachment 150851 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/13134892

[Filip Pizlo]

Created attachment 150858 [details]
the patch

Ugh more build fixes.

[WebKit Review Bot]

Attachment 150858 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1
Source/JavaScriptCore/jit/JIT.h:375:  The parameter name "returnAddress" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:376:  The parameter name "returnAddress" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:394:  The parameter name "returnAddress" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:394:  The parameter name "callFrame" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:396:  The parameter name "callFrame" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:397:  The parameter name "chain" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:397:  The parameter name "callFrame" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:398:  The parameter name "returnAddress" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:398:  The parameter name "callFrame" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:399:  The parameter name "returnAddress" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/Options.cpp:138:  Line contains only semicolon. If this should be an empty statement, use { } instead.  [whitespace/semicolon] [5]
Total errors found: 11 in 61 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Filip Pizlo]

Created attachment 150861 [details]
the patch

Maybe it'll actually build this time!

[WebKit Review Bot]

Attachment 150861 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1
Source/JavaScriptCore/jit/JIT.h:375:  The parameter name "returnAddress" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:376:  The parameter name "returnAddress" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:394:  The parameter name "returnAddress" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:394:  The parameter name "callFrame" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:396:  The parameter name "callFrame" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:397:  The parameter name "chain" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:397:  The parameter name "callFrame" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:398:  The parameter name "returnAddress" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:398:  The parameter name "callFrame" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/JIT.h:399:  The parameter name "returnAddress" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/Options.cpp:138:  Line contains only semicolon. If this should be an empty statement, use { } instead.  [whitespace/semicolon] [5]
Total errors found: 11 in 61 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Zoltan Herczeg]

I''l handle the ARM port when the patch is landed.

[Gavin Barraclough]

Comment on attachment 150861 [details]
the patch

View in context: https://bugs.webkit.org/attachment.cgi?id=150861&action=review

Agreed that that it would be nice to consider a class in the future, and even though it's not from code you added it would be good to fix the preexisting style issues re duplicate names.

> Source/JavaScriptCore/runtime/Options.cpp:121
> +#define FOR_EACH_OPTION(type_, name_, defaultValue_)    \

extra whitespace! r-!

>> Source/JavaScriptCore/runtime/Options.cpp:138
>> +#endif
> 
> Line contains only semicolon. If this should be an empty statement, use { } instead.  [whitespace/semicolon] [5]

Technically we don't allow #if 0 code, so this is on pretty shaky ground, but Im prepared to look the other way if this helps.

[Filip Pizlo]

Landed in http://trac.webkit.org/changeset/121925

[Csaba Osztrogonác]

(In reply to comment #53)
> Landed in http://trac.webkit.org/changeset/121925

FYI: I filed a bug for ARM fix - https://bugs.webkit.org/show_bug.cgi?id=90657 ,
a bug for 32 bit regression - https://bugs.webkit.org/show_bug.cgi?id=90658 and
landed a 64 bit buildfix - https://trac.webkit.org/changeset/121943

[Filip Pizlo]

(In reply to comment #54)
> (In reply to comment #53)
> > Landed in http://trac.webkit.org/changeset/121925
> 
> FYI: I filed a bug for ARM fix - https://bugs.webkit.org/show_bug.cgi?id=90657 ,
> a bug for 32 bit regression - https://bugs.webkit.org/show_bug.cgi?id=90658 and
> landed a 64 bit buildfix - https://trac.webkit.org/changeset/121943

Thanks for the 64 bit build fix!  The 32-bit regression should be fixed as of http://trac.webkit.org/changeset/121946.

Good luck with the ARM side and let me know if you need clarification of the new assembler functionality.

[Csaba Osztrogonác]

I filed one more bug for PerformanceTests/Dromaeo/cssquery-prototype.html crash - https://bugs.webkit.org/show_bug.cgi?id=90664

[Dominik Röttsches (drott)]

If I am not mistaken, the changes to JSObject.h in this page cause a lot of warnings in debug builds, like:

Source/JavaScriptCore/runtime/JSObject.h: In member function ‘void JSC::JSFinalObject::finishCreation(JSC::JSGlobalData&)’:
Source/JavaScriptCore/runtime/JSObject.h:419:61: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
Source/JavaScriptCore/runtime/JSObject.h:420:67: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]

[Dominik Röttsches (drott)]

(In reply to comment #57)
> page 

=> patch

[Arunprasad]

This breaks SH4 JIT build.

[Arunprasad]

QT SH4 bot seems to be dead. If it was active might have reported this build break.

[Premkumar Subramanian]

This patch breaks Qt Mips JIT build as well. (In reply to comment #60)
> QT SH4 bot seems to be dead. If it was active might have reported this build break.

[STEPHAN Gael]

If you don't mind Thouraya i added you here so you maybe can check out the SH4 part of the JIT ( I cannot build the latest releases of webkit because of the changes in the JIT not ported to the SH4 JIT)

Thank you!!