Commit 8b246d6d authored by mjs@apple.com's avatar mjs@apple.com

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
parent 57f4bb7f
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.
......
......@@ -1964,7 +1964,7 @@ void CTI::privateCompileMainPass()
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 @@ void CTI::privateCompileMainPass()
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 @@ void CTI::privateCompileMainPass()
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 @@ void CTI::privateCompileSlowCases()
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;
......
......@@ -348,18 +348,23 @@ void CodeBlock::dump(ExecState* exec, const Vector<Instruction>::const_iterator&
{
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;
......
......@@ -137,8 +137,8 @@ void CodeGenerator::generate()
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 @@ CodeGenerator::CodeGenerator(ProgramNode* programNode, const Debugger* debugger,
, 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 @@ CodeGenerator::CodeGenerator(FunctionBodyNode* functionBody, const Debugger* deb
, 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 @@ CodeGenerator::CodeGenerator(FunctionBodyNode* functionBody, const Debugger* deb
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 @@ CodeGenerator::CodeGenerator(EvalNode* evalNode, const Debugger* debugger, const
, 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"
......
......@@ -3375,7 +3375,7 @@ JSValue* Machine::privateExecute(ExecutionFlag flag, ExecState* exec, RegisterFi
NEXT_OPCODE;
}
BEGIN_OPCODE(op_init) {
BEGIN_OPCODE(op_enter) {
size_t i = 0;
CodeBlock* codeBlock = this->codeBlock(r);
......@@ -3388,7 +3388,7 @@ JSValue* Machine::privateExecute(ExecutionFlag flag, ExecState* exec, RegisterFi
++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 @@ JSValue* Machine::privateExecute(ExecutionFlag flag, ExecState* exec, RegisterFi
++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 @@ NEVER_INLINE void Machine::tryCTICacheGetByID(ExecState* exec, CodeBlock* codeBl
} \
} 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;
......
......@@ -143,6 +143,7 @@ namespace JSC {
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);
......
......@@ -40,9 +40,10 @@ namespace JSC {
#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) \
......
......@@ -40,7 +40,7 @@ ASSERT_CLASS_FITS_IN_CELL(JSActivation);
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))
{
}
......
......@@ -64,6 +64,8 @@ namespace JSC {
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)
......
......@@ -299,6 +299,14 @@ namespace JSC {
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);
......
......@@ -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 @@ JSGlobalData::JSGlobalData(bool isShared)
, 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())
......
......@@ -73,6 +73,8 @@ namespace JSC {
const HashTable* stringTable;
RefPtr<StructureID> nullProtoStructureID;
RefPtr<StructureID> activationStructureID;
RefPtr<StructureID> staticScopeStructureID;
RefPtr<StructureID> stringStructureID;
RefPtr<StructureID> numberStructureID;
......
......@@ -82,7 +82,7 @@ namespace JSC {
#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)
......
......@@ -44,7 +44,7 @@ namespace JSC{
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 @@ namespace JSC{
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); }
};
......
......@@ -90,7 +90,7 @@ namespace JSC {
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 };
......
......@@ -137,6 +137,7 @@ namespace JSC {
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*);
......
......@@ -35,6 +35,7 @@ namespace JSC {
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 @@ namespace JSC {
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:
......
......@@ -2192,6 +2192,7 @@ namespace JSC {
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; }
......
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.
2008-10-03 Simon Fraser <simon.fraser@apple.com>
Reviewed by Darin Adler
......
This test checks that methods called directly on primitive types get the wrapper, not the primitive, as the 'this' object.
On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
PASS (1).thisType() is 'object'
PASS (2.3).thisType() is 'object'
PASS 'xxx'.thisType() is 'object'
PASS (false).thisType() is 'object'
PASS successfullyParsed is true
TEST COMPLETE
<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
<html>
<head>
<link rel="stylesheet" href="resources/js-test-style.css">
<script src="resources/js-test-pre.js"></script>
</head>
<body>
<p id="description"></p>
<div id="console"></div>
<script src="resources/primitive-method-this.js"></script>
<script src="resources/js-test-post.js"></script>
</body>
</html>
description(
"This test checks that methods called directly on primitive types get the wrapper, not the primitive, as the 'this' object."
);
String.prototype.thisType = function() { return typeof this; };
Number.prototype.thisType = function() { return typeof this; };
Boolean.prototype.thisType = function() { return typeof this; };
shouldBe("(1).thisType()", "'object'");
shouldBe("(2.3).thisType()", "'object'");
shouldBe("'xxx'.thisType()", "'object'");
shouldBe("(false).thisType()", "'object'");
successfullyParsed = true;
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:
2008-10-03 Sam Weinig <sam@webkit.org>
Reviewed by David "The Motivator" Hyatt.
......
......@@ -459,6 +459,13 @@ sub GenerateHeader
push(@headerContent, " virtual const JSC::ClassInfo* classInfo() const { return &s_info; }\n");
push(@headerContent, " static const JSC::ClassInfo s_info;\n\n");
# Structure ID
if ($interfaceName eq "DOMWindow") {
push(@headerContent, " static PassRefPtr<JSC::StructureID> createStructureID(JSC::JSValue* proto)\n" .
" {\n" .
" return JSC::StructureID::create(proto, JSC::TypeInfo(JSC::ObjectType, JSC::ImplementsHasInstance | JSC::NeedsThisConversion));\n" .
" }\n\n");
}
# Custom mark function
push(@headerContent, " virtual void mark();\n\n") if $dataNode->extendedAttributes->{"CustomMarkFunction"};
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment