Bug 20843 – Accelerated property accesses.

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

[Gavin Barraclough]

Inline more of the array access code into the JIT code for get/put_by_val.

Accelerate get/put_by_id by speculatively inlining a disable direct access into the hot path of the code, and repatch this with the correct StructureID and property map offset once these are known.  In the case of accesses to the prototype and reading the array-length a trampoline is genertaed, and the branch to the slow-case is relinked to jump to this.

[Gavin Barraclough]

Created attachment 23416 [details]
The patch

[Gavin Barraclough]

Created attachment 23420 [details]
Sunspider results (+2.8%).

[Gavin Barraclough]

Created attachment 23421 [details]
v8 tests results - ~13% progression.

[Gavin Barraclough]

Created attachment 23422 [details]
v8 tests results - pre-patch.

[Sam Weinig]

Comment on attachment 23416 [details]
The patch

The changelog should have perf numbers as is our norm.

+// This will not sign extend from 8bit, and should be somewhere up in kernel land.
+#define INVALID_POINTER_VALUE 0xF0F0F0F0
This should be below the includes.  Is this value correct for the mac (bdash thinks perhaps not).  Can it be a const int.

structureStubCompilationInfo should be m_ structureStubCompilationInfo

structIDInstrIdx is an odd variable name.  I think it can be more verbose.


+        #define PROPERTY_ACCESS_OFFSET_PUT_STRUCTUREID 19
+        #define PROPERTY_ACCESS_OFFSET_PUT_OFFSET 34
Can these be constants instead of #defines.  Same goes for a few others just like it.

There are a bunch of c-style casts.  Please convert to use c++-style to make cpst happy (and me!).

#if USE(CTI_REPATCH_PIC) && 0 // This is not currently a performance win.  Sad face.
We usually don't commit things like this.

Please replace all TODOs with FIXMEs.

Overall, nothing popped out as being wrong, but it is a lot of code.  I think an extra pair of eyes would be good.  And for the future generations, perhaps a more detailed changelog, explaining exactly what you mean by re-patching.

r=me.  But I think it might be wise to let someone else r+ as well.

[Gavin Barraclough]

Sending        JavaScriptCore/ChangeLog
Sending        JavaScriptCore/VM/CTI.cpp
Sending        JavaScriptCore/VM/CTI.h
Sending        JavaScriptCore/VM/CodeBlock.cpp
Sending        JavaScriptCore/VM/CodeBlock.h
Sending        JavaScriptCore/VM/Machine.cpp
Sending        JavaScriptCore/VM/Machine.h
Sending        JavaScriptCore/masm/X86Assembler.h
Transmitting file data ........
Committed revision 36418.