JavaScriptCore:
2008-10-03 Maciej Stachowiak <mjs@apple.com>
Reviewed by Cameron Zwarich.
- "this" object in methods called on primitives should be wrapper object
https://bugs.webkit.org/show_bug.cgi?id=21362
I changed things so that functions which use "this" do a fast
version of toThisObject conversion if needed. Currently we miss
the conversion entirely, at least for primitive types. Using
TypeInfo and the primitive check, I made the fast case bail out
pretty fast.
This is inexplicably an 1.007x SunSpider speedup (and a wash on V8 benchmarks).
Also renamed some opcodes for clarity:
init ==> enter
init_activation ==> enter_with_activation
* VM/CTI.cpp:
(JSC::CTI::privateCompileMainPass):
(JSC::CTI::privateCompileSlowCases):
* VM/CodeBlock.cpp:
(JSC::CodeBlock::dump):
* VM/CodeGenerator.cpp:
(JSC::CodeGenerator::generate):
(JSC::CodeGenerator::CodeGenerator):
* VM/Machine.cpp:
(JSC::Machine::privateExecute):
(JSC::Machine::cti_op_convert_this):
* VM/Machine.h:
* VM/Opcode.h:
* kjs/JSActivation.cpp:
(JSC::JSActivation::JSActivation):
* kjs/JSActivation.h:
(JSC::JSActivation::createStructureID):
* kjs/JSCell.h:
(JSC::JSValue::needsThisConversion):
* kjs/JSGlobalData.cpp:
(JSC::JSGlobalData::JSGlobalData):
* kjs/JSGlobalData.h:
* kjs/JSNumberCell.h:
(JSC::JSNumberCell::createStructureID):
* kjs/JSStaticScopeObject.h:
(JSC::JSStaticScopeObject::JSStaticScopeObject):
(JSC::JSStaticScopeObject::createStructureID):
* kjs/JSString.h:
(JSC::JSString::createStructureID):
* kjs/JSValue.h:
* kjs/TypeInfo.h:
(JSC::TypeInfo::needsThisConversion):
* kjs/nodes.h:
(JSC::ScopeNode::usesThis):
WebCore:
2008-10-03 Maciej Stachowiak <mjs@apple.com>
Reviewed by Cameron Zwarich.
- "this" object in methods called on primitives should be wrapper object
https://bugs.webkit.org/show_bug.cgi?id=21362
Updated so toThis conversion for the split window is handled properly.
* bindings/scripts/CodeGeneratorJS.pm:
LayoutTests:
2008-10-03 Maciej Stachowiak <mjs@apple.com>
Reviewed by Cameron Zwarich.
- test case for: "this" object in methods called on primitives should be wrapper object
* fast/js/primitive-method-this-expected.txt: Added.
* fast/js/primitive-method-this.html: Added.
* fast/js/resources/primitive-method-this.js: Added.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@37285 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/JavaScriptCore/ChangeLog b/JavaScriptCore/ChangeLog
index c8dea35..9dc019a 100644
--- a/JavaScriptCore/ChangeLog
+++ b/JavaScriptCore/ChangeLog
@@ -1,3 +1,58 @@
+2008-10-03 Maciej Stachowiak <mjs@apple.com>
+
+ Reviewed by Cameron Zwarich.
+
+ - "this" object in methods called on primitives should be wrapper object
+ https://bugs.webkit.org/show_bug.cgi?id=21362
+
+ I changed things so that functions which use "this" do a fast
+ version of toThisObject conversion if needed. Currently we miss
+ the conversion entirely, at least for primitive types. Using
+ TypeInfo and the primitive check, I made the fast case bail out
+ pretty fast.
+
+ This is inexplicably an 1.007x SunSpider speedup (and a wash on V8 benchmarks).
+
+ Also renamed some opcodes for clarity:
+
+ init ==> enter
+ init_activation ==> enter_with_activation
+
+ * VM/CTI.cpp:
+ (JSC::CTI::privateCompileMainPass):
+ (JSC::CTI::privateCompileSlowCases):
+ * VM/CodeBlock.cpp:
+ (JSC::CodeBlock::dump):
+ * VM/CodeGenerator.cpp:
+ (JSC::CodeGenerator::generate):
+ (JSC::CodeGenerator::CodeGenerator):
+ * VM/Machine.cpp:
+ (JSC::Machine::privateExecute):
+ (JSC::Machine::cti_op_convert_this):
+ * VM/Machine.h:
+ * VM/Opcode.h:
+ * kjs/JSActivation.cpp:
+ (JSC::JSActivation::JSActivation):
+ * kjs/JSActivation.h:
+ (JSC::JSActivation::createStructureID):
+ * kjs/JSCell.h:
+ (JSC::JSValue::needsThisConversion):
+ * kjs/JSGlobalData.cpp:
+ (JSC::JSGlobalData::JSGlobalData):
+ * kjs/JSGlobalData.h:
+ * kjs/JSNumberCell.h:
+ (JSC::JSNumberCell::createStructureID):
+ * kjs/JSStaticScopeObject.h:
+ (JSC::JSStaticScopeObject::JSStaticScopeObject):
+ (JSC::JSStaticScopeObject::createStructureID):
+ * kjs/JSString.h:
+ (JSC::JSString::createStructureID):
+ * kjs/JSValue.h:
+ * kjs/TypeInfo.h:
+ (JSC::TypeInfo::needsThisConversion):
+ * kjs/nodes.h:
+ (JSC::ScopeNode::usesThis):
+
2008-10-03 Cameron Zwarich <zwarich@apple.com>
Reviewed by Maciej Stachowiak.
diff --git a/JavaScriptCore/VM/CTI.cpp b/JavaScriptCore/VM/CTI.cpp
index 7cb7106..3702946 100644
--- a/JavaScriptCore/VM/CTI.cpp
+++ b/JavaScriptCore/VM/CTI.cpp
@@ -1964,7 +1964,7 @@
i += 3;
break;
}
- case op_init: {
+ case op_enter: {
// Even though CTI doesn't use them, we initialize our constant
// registers to zap stale pointers, to avoid unnecessarily prolonging
// object lifetime and increasing GC pressure.
@@ -1975,7 +1975,7 @@
i+= 1;
break;
}
- case op_init_activation: {
+ case op_enter_with_activation: {
emitCall(i, Machine::cti_op_push_activation);
// Even though CTI doesn't use them, we initialize our constant
@@ -1993,6 +1993,17 @@
i += 1;
break;
}
+ case op_convert_this: {
+ emitGetArg(instruction[i + 1].u.operand, X86::eax);
+
+ emitJumpSlowCaseIfNotJSCell(X86::eax, i);
+ m_jit.movl_mr(OBJECT_OFFSET(JSCell, m_structureID), X86::eax, X86::edx);
+ m_jit.testl_i32m(NeedsThisConversion, OBJECT_OFFSET(StructureID, m_typeInfo.m_flags), X86::edx);
+ m_slowCases.append(SlowCaseEntry(m_jit.emitUnlinkedJnz(), i));
+
+ i += 2;
+ break;
+ }
case op_get_array_length:
case op_get_by_id_chain:
case op_get_by_id_generic:
@@ -2037,6 +2048,15 @@
for (Vector<SlowCaseEntry>::iterator iter = m_slowCases.begin(); iter != m_slowCases.end(); ++iter) {
unsigned i = iter->to;
switch (m_machine->getOpcodeID(instruction[i].u.opcode)) {
+ case op_convert_this: {
+ m_jit.link(iter->from, m_jit.label());
+ m_jit.link((++iter)->from, m_jit.label());
+ emitPutArg(X86::eax, 0);
+ emitCall(i, Machine::cti_op_convert_this);
+ emitPutResult(instruction[i + 1].u.operand);
+ i += 2;
+ break;
+ }
case op_add: {
unsigned dst = instruction[i + 1].u.operand;
unsigned src1 = instruction[i + 2].u.operand;
diff --git a/JavaScriptCore/VM/CodeBlock.cpp b/JavaScriptCore/VM/CodeBlock.cpp
index eaf94e2..dc17fc1 100644
--- a/JavaScriptCore/VM/CodeBlock.cpp
+++ b/JavaScriptCore/VM/CodeBlock.cpp
@@ -348,18 +348,23 @@
{
int location = it - begin;
switch (exec->machine()->getOpcodeID(it->u.opcode)) {
- case op_init: {
- printf("[%4d] init\n", location);
+ case op_enter: {
+ printf("[%4d] enter\n", location);
break;
}
- case op_init_activation: {
- printf("[%4d] init_activation\n", location);
+ case op_enter_with_activation: {
+ printf("[%4d] enter_with_activation\n", location);
break;
}
case op_init_arguments: {
printf("[%4d] init_arguments\n", location);
break;
}
+ case op_convert_this: {
+ int r0 = (++it)->u.operand;
+ printf("[%4d] convert_this %s\n", location, registerName(r0).c_str());
+ break;
+ }
case op_unexpected_load: {
int r0 = (++it)->u.operand;
int k0 = (++it)->u.operand;
diff --git a/JavaScriptCore/VM/CodeGenerator.cpp b/JavaScriptCore/VM/CodeGenerator.cpp
index 0234a74..bb39592 100644
--- a/JavaScriptCore/VM/CodeGenerator.cpp
+++ b/JavaScriptCore/VM/CodeGenerator.cpp
@@ -137,8 +137,8 @@
m_scopeNode->emitCode(*this);
if (m_codeType == FunctionCode && m_codeBlock->needsFullScopeChain) {
- ASSERT(globalData()->machine->getOpcodeID(m_codeBlock->instructions[0].u.opcode) == op_init);
- m_codeBlock->instructions[0] = globalData()->machine->getOpcode(op_init_activation);
+ ASSERT(globalData()->machine->getOpcodeID(m_codeBlock->instructions[0].u.opcode) == op_enter);
+ m_codeBlock->instructions[0] = globalData()->machine->getOpcode(op_enter_with_activation);
}
#ifndef NDEBUG
@@ -216,7 +216,7 @@
, m_globalData(&scopeChain.globalObject()->globalExec()->globalData())
, m_lastOpcodeID(op_end)
{
- emitOpcode(op_init);
+ emitOpcode(op_enter);
codeBlock->globalData = m_globalData;
// FIXME: Move code that modifies the global object to Machine::execute.
@@ -289,7 +289,7 @@
, m_globalData(&scopeChain.globalObject()->globalExec()->globalData())
, m_lastOpcodeID(op_end)
{
- emitOpcode(op_init);
+ emitOpcode(op_enter);
codeBlock->globalData = m_globalData;
bool usesArguments = functionBody->usesArguments();
@@ -321,6 +321,11 @@
m_thisRegister.setIndex(m_nextParameter);
++m_nextParameter;
++m_codeBlock->numParameters;
+
+ if (functionBody->usesThis()) {
+ emitOpcode(op_convert_this);
+ instructions().append(m_thisRegister.index());
+ }
for (size_t i = 0; i < parameterCount; ++i)
addParameter(parameters[i]);
@@ -342,7 +347,7 @@
, m_globalData(&scopeChain.globalObject()->globalExec()->globalData())
, m_lastOpcodeID(op_end)
{
- emitOpcode(op_init);
+ emitOpcode(op_enter);
codeBlock->globalData = m_globalData;
m_codeBlock->numParameters = 1; // Allocate space for "this"
diff --git a/JavaScriptCore/VM/Machine.cpp b/JavaScriptCore/VM/Machine.cpp
index a14ee1b..ed40eb5 100644
--- a/JavaScriptCore/VM/Machine.cpp
+++ b/JavaScriptCore/VM/Machine.cpp
@@ -3375,7 +3375,7 @@
NEXT_OPCODE;
}
- BEGIN_OPCODE(op_init) {
+ BEGIN_OPCODE(op_enter) {
size_t i = 0;
CodeBlock* codeBlock = this->codeBlock(r);
@@ -3388,7 +3388,7 @@
++vPC;
NEXT_OPCODE;
}
- BEGIN_OPCODE(op_init_activation) {
+ BEGIN_OPCODE(op_enter_with_activation) {
size_t i = 0;
CodeBlock* codeBlock = this->codeBlock(r);
@@ -3405,6 +3405,15 @@
++vPC;
NEXT_OPCODE;
}
+ BEGIN_OPCODE(op_convert_this) {
+ int thisRegister = (++vPC)->u.operand;
+ JSValue* thisVal = r[thisRegister].getJSValue();
+ if (thisVal->needsThisConversion())
+ r[thisRegister] = thisVal->toThisObject(exec);
+
+ ++vPC;
+ NEXT_OPCODE;
+ }
BEGIN_OPCODE(op_init_arguments) {
JSValue* activation = r[RegisterFile::OptionalCalleeActivation].getJSValue();
Arguments* arguments;
@@ -4200,6 +4209,17 @@
} \
} while (0)
+
+JSValue* Machine::cti_op_convert_this(CTI_ARGS)
+{
+ JSValue* v1 = ARG_src1;
+ ExecState* exec = ARG_exec;
+
+ JSObject* result = v1->toThisObject(exec);
+ VM_CHECK_EXCEPTION_AT_END();
+ return result;
+}
+
void Machine::cti_op_end(CTI_ARGS)
{
Register* r = ARG_r;
diff --git a/JavaScriptCore/VM/Machine.h b/JavaScriptCore/VM/Machine.h
index 4f2057d..c2a9d36 100644
--- a/JavaScriptCore/VM/Machine.h
+++ b/JavaScriptCore/VM/Machine.h
@@ -143,6 +143,7 @@
static void SFX_CALL cti_timeout_check(CTI_ARGS);
+ static JSValue* SFX_CALL cti_op_convert_this(CTI_ARGS);
static void SFX_CALL cti_op_end(CTI_ARGS);
static JSValue* SFX_CALL cti_op_add(CTI_ARGS);
static JSValue* SFX_CALL cti_op_pre_inc(CTI_ARGS);
diff --git a/JavaScriptCore/VM/Opcode.h b/JavaScriptCore/VM/Opcode.h
index 425d3c3..f1c9029 100644
--- a/JavaScriptCore/VM/Opcode.h
+++ b/JavaScriptCore/VM/Opcode.h
@@ -40,9 +40,10 @@
#define DUMP_OPCODE_STATS 0
#define FOR_EACH_OPCODE_ID(macro) \
- macro(op_init) \
- macro(op_init_activation) \
+ macro(op_enter) \
+ macro(op_enter_with_activation) \
macro(op_init_arguments) \
+ macro(op_convert_this) \
\
macro(op_unexpected_load) \
macro(op_new_object) \
diff --git a/JavaScriptCore/kjs/JSActivation.cpp b/JavaScriptCore/kjs/JSActivation.cpp
index b83a370..2905510 100644
--- a/JavaScriptCore/kjs/JSActivation.cpp
+++ b/JavaScriptCore/kjs/JSActivation.cpp
@@ -40,7 +40,7 @@
const ClassInfo JSActivation::info = { "JSActivation", 0, 0, 0 };
JSActivation::JSActivation(ExecState* exec, PassRefPtr<FunctionBodyNode> functionBody, Register* registers)
- : Base(exec->globalData().nullProtoStructureID, new JSActivationData(functionBody, registers))
+ : Base(exec->globalData().activationStructureID, new JSActivationData(functionBody, registers))
{
}
diff --git a/JavaScriptCore/kjs/JSActivation.h b/JavaScriptCore/kjs/JSActivation.h
index 7361a96..ef45f27 100644
--- a/JavaScriptCore/kjs/JSActivation.h
+++ b/JavaScriptCore/kjs/JSActivation.h
@@ -64,6 +64,8 @@
virtual const ClassInfo* classInfo() const { return &info; }
static const ClassInfo info;
+ static PassRefPtr<StructureID> createStructureID(JSValue* proto) { return StructureID::create(proto, TypeInfo(ObjectType, NeedsThisConversion)); }
+
private:
struct JSActivationData : public JSVariableObjectData {
JSActivationData(PassRefPtr<FunctionBodyNode> functionBody, Register* registers)
diff --git a/JavaScriptCore/kjs/JSCell.h b/JavaScriptCore/kjs/JSCell.h
index 6b300bd..bced0cd 100644
--- a/JavaScriptCore/kjs/JSCell.h
+++ b/JavaScriptCore/kjs/JSCell.h
@@ -299,6 +299,14 @@
return asCell()->toThisObject(exec);
}
+ inline bool JSValue::needsThisConversion() const
+ {
+ if (UNLIKELY(JSImmediate::isImmediate(this)))
+ return true;
+
+ return asCell()->structureID()->typeInfo().needsThisConversion();
+ }
+
inline UString JSValue::toThisString(ExecState* exec) const
{
return JSImmediate::isImmediate(this) ? JSImmediate::toString(this) : asCell()->toThisString(exec);
diff --git a/JavaScriptCore/kjs/JSGlobalData.cpp b/JavaScriptCore/kjs/JSGlobalData.cpp
index 4636086..7633731 100644
--- a/JavaScriptCore/kjs/JSGlobalData.cpp
+++ b/JavaScriptCore/kjs/JSGlobalData.cpp
@@ -31,8 +31,10 @@
#include "ArgList.h"
#include "CommonIdentifiers.h"
+#include "JSActivation.h"
#include "JSClassRef.h"
#include "JSLock.h"
+#include "JSStaticScopeObject.h"
#include "Machine.h"
#include "Parser.h"
#include "collector.h"
@@ -76,6 +78,8 @@
, stringTable(&JSC::stringTable)
#endif
, nullProtoStructureID(JSObject::createStructureID(jsNull()))
+ , activationStructureID(JSActivation::createStructureID(jsNull()))
+ , staticScopeStructureID(JSStaticScopeObject::createStructureID(jsNull()))
, stringStructureID(JSString::createStructureID(jsNull()))
, numberStructureID(JSNumberCell::createStructureID(jsNull()))
, identifierTable(createIdentifierTable())
diff --git a/JavaScriptCore/kjs/JSGlobalData.h b/JavaScriptCore/kjs/JSGlobalData.h
index be6fa97..937a785 100644
--- a/JavaScriptCore/kjs/JSGlobalData.h
+++ b/JavaScriptCore/kjs/JSGlobalData.h
@@ -73,6 +73,8 @@
const HashTable* stringTable;
RefPtr<StructureID> nullProtoStructureID;
+ RefPtr<StructureID> activationStructureID;
+ RefPtr<StructureID> staticScopeStructureID;
RefPtr<StructureID> stringStructureID;
RefPtr<StructureID> numberStructureID;
diff --git a/JavaScriptCore/kjs/JSNumberCell.h b/JavaScriptCore/kjs/JSNumberCell.h
index ab257d2..1116651 100644
--- a/JavaScriptCore/kjs/JSNumberCell.h
+++ b/JavaScriptCore/kjs/JSNumberCell.h
@@ -82,7 +82,7 @@
#endif
}
- static PassRefPtr<StructureID> createStructureID(JSValue* proto) { return StructureID::create(proto, TypeInfo(NumberType)); }
+ static PassRefPtr<StructureID> createStructureID(JSValue* proto) { return StructureID::create(proto, TypeInfo(NumberType, NeedsThisConversion)); }
private:
JSNumberCell(JSGlobalData* globalData, double value)
diff --git a/JavaScriptCore/kjs/JSStaticScopeObject.h b/JavaScriptCore/kjs/JSStaticScopeObject.h
index cc3bf02..b152862 100644
--- a/JavaScriptCore/kjs/JSStaticScopeObject.h
+++ b/JavaScriptCore/kjs/JSStaticScopeObject.h
@@ -44,7 +44,7 @@
public:
JSStaticScopeObject(ExecState* exec, const Identifier& ident, JSValue* value, unsigned attributes)
- : JSVariableObject(exec->globalData().nullProtoStructureID, new JSStaticScopeObjectData())
+ : JSVariableObject(exec->globalData().staticScopeStructureID, new JSStaticScopeObjectData())
{
d()->registerStore = value;
symbolTable().add(ident.ustring().rep(), SymbolTableEntry(-1, attributes));
@@ -58,6 +58,8 @@
virtual void put(ExecState*, const Identifier&, JSValue*, PutPropertySlot&);
void putWithAttributes(ExecState*, const Identifier&, JSValue*, unsigned attributes);
+ static PassRefPtr<StructureID> createStructureID(JSValue* proto) { return StructureID::create(proto, TypeInfo(ObjectType, NeedsThisConversion)); }
+
private:
JSStaticScopeObjectData* d() { return static_cast<JSStaticScopeObjectData*>(JSVariableObject::d); }
};
diff --git a/JavaScriptCore/kjs/JSString.h b/JavaScriptCore/kjs/JSString.h
index e502c11..d7c37bf 100644
--- a/JavaScriptCore/kjs/JSString.h
+++ b/JavaScriptCore/kjs/JSString.h
@@ -90,7 +90,7 @@
bool canGetIndex(unsigned i) { return i < static_cast<unsigned>(m_value.size()); }
JSString* getIndex(JSGlobalData*, unsigned);
- static PassRefPtr<StructureID> createStructureID(JSValue* proto) { return StructureID::create(proto, TypeInfo(StringType)); }
+ static PassRefPtr<StructureID> createStructureID(JSValue* proto) { return StructureID::create(proto, TypeInfo(StringType, NeedsThisConversion)); }
private:
enum VPtrStealingHackType { VPtrStealingHack };
diff --git a/JavaScriptCore/kjs/JSValue.h b/JavaScriptCore/kjs/JSValue.h
index d901c0b..d0e4513 100644
--- a/JavaScriptCore/kjs/JSValue.h
+++ b/JavaScriptCore/kjs/JSValue.h
@@ -137,6 +137,7 @@
bool deleteProperty(ExecState*, const Identifier& propertyName);
bool deleteProperty(ExecState*, unsigned propertyName);
+ bool needsThisConversion() const;
JSObject* toThisObject(ExecState*) const;
UString toThisString(ExecState*) const;
JSString* toThisJSString(ExecState*);
diff --git a/JavaScriptCore/kjs/TypeInfo.h b/JavaScriptCore/kjs/TypeInfo.h
index ac86d9d..477e0f2 100644
--- a/JavaScriptCore/kjs/TypeInfo.h
+++ b/JavaScriptCore/kjs/TypeInfo.h
@@ -35,6 +35,7 @@
static const unsigned MasqueradesAsUndefined = 1;
static const unsigned ImplementsHasInstance = 1 << 1;
static const unsigned OverridesHasInstance = 1 << 2;
+ static const unsigned NeedsThisConversion = 1 << 3;
class TypeInfo {
friend class CTI;
@@ -46,6 +47,7 @@
bool masqueradesAsUndefined() const { return m_flags & MasqueradesAsUndefined; }
bool implementsHasInstance() const { return m_flags & ImplementsHasInstance; }
bool overridesHasInstance() const { return m_flags & OverridesHasInstance; }
+ bool needsThisConversion() const { return m_flags & NeedsThisConversion; }
unsigned flags() const { return m_flags; }
private:
diff --git a/JavaScriptCore/kjs/nodes.h b/JavaScriptCore/kjs/nodes.h
index 8162993..fc5e5a7 100644
--- a/JavaScriptCore/kjs/nodes.h
+++ b/JavaScriptCore/kjs/nodes.h
@@ -2192,6 +2192,7 @@
bool containsClosures() const { return m_features & ClosureFeature; }
bool usesArguments() const { return m_features & ArgumentsFeature; }
void setUsesArguments() { m_features |= ArgumentsFeature; }
+ bool usesThis() const { return m_features & ThisFeature; }
VarStack& varStack() { return m_varStack; }
FunctionStack& functionStack() { return m_functionStack; }
