Summary: | Accelerated property accesses. | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Gavin Barraclough <barraclough> | ||||||||||
Component: | JavaScriptCore | Assignee: | Gavin Barraclough <barraclough> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | ||||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | Mac | ||||||||||||
OS: | OS X 10.5 | ||||||||||||
Attachments: |
|
Description
Gavin Barraclough
2008-09-14 14:51:18 PDT
Created attachment 23416 [details]
The patch
Created attachment 23420 [details]
Sunspider results (+2.8%).
Created attachment 23421 [details]
v8 tests results - ~13% progression.
Created attachment 23422 [details]
v8 tests results - pre-patch.
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.
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. |