Bug 54786 – Devirtualize JSCell::classInfo()

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Gavin Barraclough]

Instead of making a virtual function call, add a pointer to the ClassInfo onto Structure.

This removes a virtual function call, and paves the way towards removing all the createStructure methods, and StructureFlags/AnonymousSlotCount properties (these should be able to move onto ClassInfo).

Calls to Structure::create must now pass a pointer to the ClassInfo for the structure.  All objects now have a ClassInfo pointer, non-object cell types still do not.

[Gavin Barraclough]

Created attachment 83045 [details]
The patch, this shows a small sunspider regression, but a similar v8 progression - a wash overall.

[Gavin Barraclough]

Oooops, ignore patch title, auto complete kicked in!
(tested as no perf impact on JSC).

[WebKit Review Bot]

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

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/API/JSCallbackConstr..." exit_code: 1

Source/WebCore/bridge/c/CRuntimeObject.cpp:32:  Alphabetical sorting problem.  [build/include_order] [4]
Source/JavaScriptCore/runtime/Structure.h:149:  The parameter name "classInfo" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/Structure.h:150:  The parameter name "structure" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WebCore/bridge/jni/jsc/JavaRuntimeObject.cpp:28:  Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
Source/WebCore/bridge/jni/jsc/JavaRuntimeObject.cpp:29:  Alphabetical sorting problem.  [build/include_order] [4]
Source/JavaScriptCore/API/JSObjectRef.cpp:343:  An else if statement should be written as an if statement when the prior "if" concludes with a return, break, continue or goto statement.  [readability/control_flow] [4]
Source/JavaScriptCore/API/JSObjectRef.cpp:355:  An else if statement should be written as an if statement when the prior "if" concludes with a return, break, continue or goto statement.  [readability/control_flow] [4]
Source/JavaScriptCore/runtime/JSObject.h:325:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/JavaScriptCore/runtime/JSByteArray.cpp:55:  Local variables should never be PassRefPtr (see http://webkit.org/coding/RefPtr.html).  [readability/pass_ptr] [5]
Source/WebCore/bridge/jsc/BridgeJSC.cpp:34:  Alphabetical sorting problem.  [build/include_order] [4]
Source/JavaScriptCore/runtime/JSByteArray.h:71:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/API/JSValueRef.cpp:132:  An else if statement should be written as an if statement when the prior "if" concludes with a return, break, continue or goto statement.  [readability/control_flow] [4]
Total errors found: 12 in 144 files


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

[Sam Weinig]

Comment on attachment 83045 [details]
The patch, this shows a small sunspider regression, but a similar v8 progression - a wash overall.

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

> Source/JavaScriptCore/ChangeLog:266
> +        (JSC::Structure::toDictionaryTransition):
> +        * runtime/Structure.h:
> +        (JSC::Structure::create):
> +        (JSC::Structure::classInfo):
> +

I am not sure how much of this wall of text is really necessary.

> Source/JavaScriptCore/jit/JITOpcodes.cpp:587
> -    Jump notObject = branch8(NotEqual, Address(regT2, OBJECT_OFFSETOF(Structure, m_typeInfo) + OBJECT_OFFSETOF(TypeInfo, m_type)), Imm32(ObjectType));
> +    Jump notObject = branch8(NotEqual, Address(regT2, OBJECT_OFFSETOF(Structure, m_typeInfo.m_type)), Imm32(ObjectType));

This seems unrelated.

> Source/JavaScriptCore/runtime/InternalFunction.cpp:32
> +void InternalFunction::vtableFix()

I like the name vtableAnchor a bit better.

> Source/JavaScriptCore/runtime/JSByteArray.cpp:56
> +    PassRefPtr<Structure> result = Structure::create(prototype, TypeInfo(ObjectType, StructureFlags), AnonymousSlotCount, classInfo);
>      return result;

This should just return directly.

[Build Bot]

Attachment 83045 [details] did not build on win:
Build output: http://queues.webkit.org/results/7939214

[Early Warning System Bot]

Attachment 83045 [details] did not build on qt:
Build output: http://queues.webkit.org/results/7937291

[WebKit Review Bot]

Attachment 83045 [details] did not build on mac:
Build output: http://queues.webkit.org/results/7940121

[Gavin Barraclough]

Fixed in r79132 (+ many build fixes!)