WebKit Bugzilla
New
Browse
Search+
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
[patch]
Patch
bug-74633-20120118193027.patch (text/plain), 18.98 KB, created by
Andy Wingo
on 2012-01-18 10:30:29 PST
(
hide
)
Description:
Patch
Filename:
MIME Type:
Creator:
Andy Wingo
Created:
2012-01-18 10:30:29 PST
Size:
18.98 KB
patch
obsolete
>Subversion Revision: 105249 >diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog >index bb90396ddd5976b6efb0d600596ef841984f6c20..8356536ef38dfe580507916083385e78189bf411 100644 >--- a/Source/JavaScriptCore/ChangeLog >+++ b/Source/JavaScriptCore/ChangeLog >@@ -1,5 +1,56 @@ > 2011-12-15 Andy Wingo <wingo@igalia.com> > >+ Allow JSStaticScope to bind more than one variable >+ https://bugs.webkit.org/show_bug.cgi?id=74633 >+ >+ This patch generalizes JSStaticScope to be able to bind more than >+ one variable. As the symbol table could grow large, instead of >+ copying it all the time, we refcount it. A "template" >+ JSStaticScope can beget other scope objects that share the symbol >+ table, but have their own storage. It also changes the >+ push_new_scope op to take a template scope as an argument instead >+ of an identifier. Eventually we will remove the "value" arg from >+ push_new_scope, and instead bind the variable with an >+ op_put_scoped_var. >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ * runtime/JSStaticScopeObject.h: >+ (JSC::JSStaticScopeObject::create): Have a new variant that makes >+ an "empty" scope -- a scope template. >+ (JSC::JSStaticScopeObject::addVariable): Call this method to add >+ slots (symbol table entries) to scope templates. >+ (JSC::JSStaticScopeObject::clone): This method will clone a scope >+ template and allocate storage for its variables, though without >+ allocation in the common case of only binding one variable. >+ (JSC::JSStaticScopeObject::JSStaticScopeObject): Adapt to take a >+ reference on a shared symbol table. >+ * runtime/JSStaticScopeObject.cpp: >+ (JSC::JSStaticScopeObject::visitChildren): Visit all the >+ registers! >+ * bytecode/CodeBlock.cpp (JSC::CodeBlock::dump): Update for new >+ op_push_new_scope. >+ * bytecompiler/BytecodeGenerator.h: >+ * bytecompiler/BytecodeGenerator.cpp: >+ (JSC::BytecodeGenerator::emitPushNewScope): Take a register for >+ the scope template, instead of an ID. >+ * bytecompiler/NodesCodegen.cpp: >+ (JSC::TryNode::emitBytecode): Create a scope template, emit a >+ load, and plumb the resulting register down to emitPushNewScope. >+ * interpreter/Interpreter.cpp: >+ (JSC::Interpreter::createExceptionScope): Instead of making a new >+ scope from scratch, clone one from the incoming template. >+ (JSC::Interpreter::privateexecute): Adapt a comment. >+ * jit/JITStubs.cpp (JSC::cti_op_push_new_scope): Instead of taking >+ the identifier as the first argument, take the scope template. >+ * jit/JITOpcodes.cpp (JSC::JIT::emit_op_push_new_scope): >+ * jit/JITOpcodes32_64.cpp (JSC::JIT::emit_op_push_new_scope): >+ Adapt to cti_op_push_new_scope change. >+ * runtime/JSObject.h (JSC::JSObject::isStaticScopeObject): New >+ helper. >+ >+2011-12-15 Andy Wingo <wingo@igalia.com> >+ > Avoid double-lookup when setting static-scoped vars in strict mode > https://bugs.webkit.org/show_bug.cgi?id=74628 > >diff --git a/Source/JavaScriptCore/bytecode/CodeBlock.cpp b/Source/JavaScriptCore/bytecode/CodeBlock.cpp >index 8f283d826cf0722d0fc862668435c2d259254c53..7869e99259b5ec714448f2b134e035fe3506219b 100644 >--- a/Source/JavaScriptCore/bytecode/CodeBlock.cpp >+++ b/Source/JavaScriptCore/bytecode/CodeBlock.cpp >@@ -1219,9 +1219,9 @@ void CodeBlock::dump(ExecState* exec, const Vector<Instruction>::const_iterator& > } > case op_push_new_scope: { > int r0 = (++it)->u.operand; >- int id0 = (++it)->u.operand; > int r1 = (++it)->u.operand; >- printf("[%4d] push_new_scope \t%s, %s, %s\n", location, registerName(exec, r0).data(), idName(id0, m_identifiers[id0]).data(), registerName(exec, r1).data()); >+ int r2 = (++it)->u.operand; >+ printf("[%4d] push_new_scope \t%s, %s, %s\n", location, registerName(exec, r0).data(), registerName(exec, r1).data(), registerName(exec, r2).data()); > break; > } > case op_jmp_scopes: { >diff --git a/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp b/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp >index b8ae042b85c93b43b7e29d689ef84cd0985b7f1e..251293cd9cc4697f21e98f1fa8a2e35660508316 100644 >--- a/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp >+++ b/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp >@@ -2233,8 +2233,9 @@ void BytecodeGenerator::emitSubroutineReturn(RegisterID* retAddrSrc) > instructions().append(retAddrSrc->index()); > } > >-void BytecodeGenerator::emitPushNewScope(RegisterID* dst, const Identifier& property, RegisterID* value) >+void BytecodeGenerator::emitPushNewScope(RegisterID* dst, RegisterID *scopeTemplate, RegisterID* value) > { >+ // FIXME: Track the scope so that findScopedProperty can DTRT. > ControlFlowContext context; > context.isFinallyBlock = false; > m_scopeContextStack.append(context); >@@ -2242,7 +2243,7 @@ void BytecodeGenerator::emitPushNewScope(RegisterID* dst, const Identifier& prop > > emitOpcode(op_push_new_scope); > instructions().append(dst->index()); >- instructions().append(addConstant(property)); >+ instructions().append(scopeTemplate->index()); > instructions().append(value->index()); > } > >diff --git a/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h b/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h >index 6daf27715958a9f3859f0f1ddacd1fc8597e6e81..67b311642b6a02db01d06d4adb10b2c7731dc583 100644 >--- a/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h >+++ b/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h >@@ -442,7 +442,7 @@ namespace JSC { > > void emitThrowReferenceError(const UString& message); > >- void emitPushNewScope(RegisterID* dst, const Identifier& property, RegisterID* value); >+ void emitPushNewScope(RegisterID* dst, RegisterID *scopeTemplate, RegisterID* value); > > RegisterID* emitPushScope(RegisterID* scope); > void emitPopScope(); >diff --git a/Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp b/Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp >index d6f09e9494437c5a9b5c81822e9903f3fdeac91c..6a74ad7f74bbdb3e75c5ca75bd4183910ea13534 100644 >--- a/Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp >+++ b/Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp >@@ -1922,8 +1922,12 @@ RegisterID* TryNode::emitBytecode(BytecodeGenerator& generator, RegisterID* dst) > generator.emitPutById(dynamicScopeObject.get(), m_exceptionIdent, exceptionRegister.get()); > generator.emitMove(exceptionRegister.get(), dynamicScopeObject.get()); > generator.emitPushScope(exceptionRegister.get()); >- } else >- generator.emitPushNewScope(exceptionRegister.get(), m_exceptionIdent, exceptionRegister.get()); >+ } else { >+ JSStaticScopeObject* scopeTemplate = JSStaticScopeObject::create(generator.scopeChain()->globalObject->globalExec()); >+ scopeTemplate->addVariable(m_exceptionIdent, DontDelete); >+ RefPtr<RegisterID> templ = generator.emitLoad(0, JSValue(scopeTemplate)); >+ generator.emitPushNewScope(exceptionRegister.get(), templ.get(), exceptionRegister.get()); >+ } > generator.emitNode(dst, m_catchBlock); > generator.emitPopScope(); > generator.emitLabel(catchEndLabel.get()); >diff --git a/Source/JavaScriptCore/interpreter/Interpreter.cpp b/Source/JavaScriptCore/interpreter/Interpreter.cpp >index d30679f1f910f2dc36e2f0f7038fb8827f58bf74..e98a12c72865e368e034e88893931d9bd6e67f98 100644 >--- a/Source/JavaScriptCore/interpreter/Interpreter.cpp >+++ b/Source/JavaScriptCore/interpreter/Interpreter.cpp >@@ -1439,13 +1439,16 @@ NEVER_INLINE void Interpreter::debug(CallFrame* callFrame, DebugHookID debugHook > NEVER_INLINE ScopeChainNode* Interpreter::createExceptionScope(CallFrame* callFrame, const Instruction* vPC) > { > int dst = vPC[1].u.operand; >- CodeBlock* codeBlock = callFrame->codeBlock(); >- Identifier& property = codeBlock->identifier(vPC[2].u.operand); >- JSValue value = callFrame->r(vPC[3].u.operand).jsValue(); >- JSObject* scope = JSStaticScopeObject::create(callFrame, property, value, DontDelete); >- callFrame->uncheckedR(dst) = JSValue(scope); >+ int templ = vPC[2].u.operand; >+ int value = vPC[3].u.operand; >+ >+ ASSERT(callFrame->r(templ).jsValue().isObject()); >+ JSObject* scopeTemplate = asObject(callFrame->r(templ).jsValue()); >+ ASSERT(scopeTemplate->isStaticScopeObject()); >+ JSStaticScopeObject* newScope = static_cast<JSStaticScopeObject*>(scopeTemplate)->clone(callFrame, callFrame->r(value).jsValue()); >+ callFrame->uncheckedR(dst) = newScope; > >- return callFrame->scopeChain()->push(scope); >+ return callFrame->scopeChain()->push(newScope); > } > > NEVER_INLINE void Interpreter::tryCachePutByID(CallFrame* callFrame, CodeBlock* codeBlock, Instruction* vPC, JSValue baseValue, const PutPropertySlot& slot) >@@ -4888,11 +4891,11 @@ skip_id_custom_self: > goto *(&&skip_new_scope); > #endif > DEFINE_OPCODE(op_push_new_scope) { >- /* new_scope dst(r) property(id) value(r) >+ /* new_scope dst(r) templ(r) value(r) > >- Constructs a new StaticScopeObject with property set to value. That scope >- object is then pushed onto the ScopeChain. The scope object is then stored >- in dst for GC. >+ Constructs a new StaticScopeObject whose one property is set to >+ value. That scope object is then pushed onto the ScopeChain. The >+ scope object is then stored in dst for GC. > */ > callFrame->setScopeChain(createExceptionScope(callFrame, vPC)); > >diff --git a/Source/JavaScriptCore/jit/JITOpcodes.cpp b/Source/JavaScriptCore/jit/JITOpcodes.cpp >index f5be279a629c3e1fc6d562e49aab3faea0845954..221a9b085d87d1959ceb047c77e2c108c934b57b 100644 >--- a/Source/JavaScriptCore/jit/JITOpcodes.cpp >+++ b/Source/JavaScriptCore/jit/JITOpcodes.cpp >@@ -1012,7 +1012,7 @@ void JIT::emit_op_to_jsnumber(Instruction* currentInstruction) > void JIT::emit_op_push_new_scope(Instruction* currentInstruction) > { > JITStubCall stubCall(this, cti_op_push_new_scope); >- stubCall.addArgument(TrustedImmPtr(&m_codeBlock->identifier(currentInstruction[2].u.operand))); >+ stubCall.addArgument(currentInstruction[2].u.operand, regT1); > stubCall.addArgument(currentInstruction[3].u.operand, regT2); > stubCall.call(currentInstruction[1].u.operand); > } >diff --git a/Source/JavaScriptCore/jit/JITOpcodes32_64.cpp b/Source/JavaScriptCore/jit/JITOpcodes32_64.cpp >index 9d1cbce8de96c1dd1a7d2cb0f5638f8aa6dd2989..831f9f6a4b1b81269cf1db7b7cdc341d97b0e4e7 100644 >--- a/Source/JavaScriptCore/jit/JITOpcodes32_64.cpp >+++ b/Source/JavaScriptCore/jit/JITOpcodes32_64.cpp >@@ -1331,7 +1331,7 @@ void JIT::emitSlow_op_to_jsnumber(Instruction* currentInstruction, Vector<SlowCa > void JIT::emit_op_push_new_scope(Instruction* currentInstruction) > { > JITStubCall stubCall(this, cti_op_push_new_scope); >- stubCall.addArgument(TrustedImmPtr(&m_codeBlock->identifier(currentInstruction[2].u.operand))); >+ stubCall.addArgument(currentInstruction[2].u.operand); > stubCall.addArgument(currentInstruction[3].u.operand); > stubCall.call(currentInstruction[1].u.operand); > } >diff --git a/Source/JavaScriptCore/jit/JITStubs.cpp b/Source/JavaScriptCore/jit/JITStubs.cpp >index ec4ae1c2eb910258be28299bbe290972b726d796..7e5e0ab50e4f7252532a8b7c03bbb2e27f15c88c 100644 >--- a/Source/JavaScriptCore/jit/JITStubs.cpp >+++ b/Source/JavaScriptCore/jit/JITStubs.cpp >@@ -3395,11 +3395,15 @@ DEFINE_STUB_FUNCTION(JSObject*, op_push_new_scope) > { > STUB_INIT_STACK_FRAME(stackFrame); > >- JSObject* scope = JSStaticScopeObject::create(stackFrame.callFrame, stackFrame.args[0].identifier(), stackFrame.args[1].jsValue(), DontDelete); >+ ASSERT(stackFrame.args[0].jsValue().isObject()); >+ JSObject* scopeTemplate = asObject(stackFrame.args[0].jsValue()); >+ ASSERT(scopeTemplate->isStaticScopeObject()); > > CallFrame* callFrame = stackFrame.callFrame; >- callFrame->setScopeChain(callFrame->scopeChain()->push(scope)); >- return scope; >+ JSValue value = stackFrame.args[1].jsValue(); >+ JSStaticScopeObject* newScope = static_cast<JSStaticScopeObject*>(scopeTemplate)->clone(callFrame, value); >+ callFrame->setScopeChain(callFrame->scopeChain()->push(newScope)); >+ return newScope; > } > > DEFINE_STUB_FUNCTION(void, op_jmp_scopes) >diff --git a/Source/JavaScriptCore/runtime/JSObject.h b/Source/JavaScriptCore/runtime/JSObject.h >index 6d45e82d94d2fe4bff34fcfa69a98e35b6934f59..e12e9f0817abe6c784979867c5111b49d1c7e571 100644 >--- a/Source/JavaScriptCore/runtime/JSObject.h >+++ b/Source/JavaScriptCore/runtime/JSObject.h >@@ -205,6 +205,7 @@ namespace JSC { > bool isGlobalObject() const; > bool isVariableObject() const; > bool isActivationObject() const; >+ bool isStaticScopeObject() const; > bool isErrorInstance() const; > bool isGlobalThis() const; > >@@ -434,6 +435,11 @@ inline bool JSObject::isActivationObject() const > return structure()->typeInfo().type() == ActivationObjectType; > } > >+inline bool JSObject::isStaticScopeObject() const >+{ >+ return structure()->typeInfo().type() == StaticScopeObjectType; >+} >+ > inline bool JSObject::isErrorInstance() const > { > return structure()->typeInfo().type() == ErrorInstanceType; >diff --git a/Source/JavaScriptCore/runtime/JSStaticScopeObject.cpp b/Source/JavaScriptCore/runtime/JSStaticScopeObject.cpp >index e8705a4fb323d77a00c83afd41db9cac738ca0bf..6cca71ad1a37a1cdae3bf0bcfd9c623409819c1d 100644 >--- a/Source/JavaScriptCore/runtime/JSStaticScopeObject.cpp >+++ b/Source/JavaScriptCore/runtime/JSStaticScopeObject.cpp >@@ -46,7 +46,8 @@ void JSStaticScopeObject::visitChildren(JSCell* cell, SlotVisitor& visitor) > COMPILE_ASSERT(StructureFlags & OverridesVisitChildren, OverridesVisitChildrenWithoutSettingFlag); > ASSERT(thisObject->structure()->typeInfo().overridesVisitChildren()); > JSVariableObject::visitChildren(thisObject, visitor); >- visitor.append(&thisObject->m_registerStore); >+ if (thisObject->m_registers) >+ visitor.appendValues(thisObject->m_registers, thisObject->m_symbolTable->size()); > } > > JSObject* JSStaticScopeObject::toThisObject(JSCell*, ExecState* exec) >diff --git a/Source/JavaScriptCore/runtime/JSStaticScopeObject.h b/Source/JavaScriptCore/runtime/JSStaticScopeObject.h >index bbf03a3473a25769d2b4259b750f6bf7ffd40f89..0984b630232e69ecd5eed561246b32e6d7123a08 100644 >--- a/Source/JavaScriptCore/runtime/JSStaticScopeObject.h >+++ b/Source/JavaScriptCore/runtime/JSStaticScopeObject.h >@@ -34,13 +34,57 @@ namespace JSC{ > public: > typedef JSVariableObject Base; > >- static JSStaticScopeObject* create(ExecState* exec, const Identifier& identifier, JSValue value, unsigned attributes) >+ static JSStaticScopeObject* create(ExecState* exec) > { >- JSStaticScopeObject* scopeObject = new (NotNull, allocateCell<JSStaticScopeObject>(*exec->heap())) JSStaticScopeObject(exec); >- scopeObject->finishCreation(exec, identifier, value, attributes); >+ RefPtr<SharedSymbolTable> table = SharedSymbolTable::create(); >+ JSStaticScopeObject* scopeObject = new (NotNull, allocateCell<JSStaticScopeObject>(*exec->heap())) JSStaticScopeObject(exec, table.release()); >+ scopeObject->finishCreation(exec->globalData()); > return scopeObject; > } > >+ static JSStaticScopeObject* create(ExecState* exec, const Identifier& name, JSValue value, unsigned attributes) >+ { >+ JSStaticScopeObject* scopeObject = create(exec); >+ scopeObject->addVariable(name, attributes); >+ scopeObject->m_registers = &scopeObject->m_singleRegisterStore; >+ scopeObject->m_singleRegisterStore.set(exec->globalData(), scopeObject, value); >+ return scopeObject; >+ } >+ >+ JSStaticScopeObject* clone(ExecState* exec) >+ { >+ JSStaticScopeObject *scopeObject = new (allocateCell<JSStaticScopeObject>(*exec->heap())) JSStaticScopeObject(exec, m_symbolTable); >+ if (m_symbolTable->size() == 1) { >+ scopeObject->m_registers = &scopeObject->m_singleRegisterStore; >+ scopeObject->m_singleRegisterStore.setUndefined(); >+ } else { >+ OwnArrayPtr<WriteBarrier<Unknown> > registerArray = adoptArrayPtr(new WriteBarrier<Unknown>[m_symbolTable->size()]); >+ for (int i = 0; i < m_symbolTable->size(); i++) >+ registerArray[i].setUndefined(); >+ WriteBarrier<Unknown> *registers = registerArray.get(); >+ scopeObject->setRegisters(registers, registerArray.release()); >+ } >+ scopeObject->finishCreation(exec->globalData()); >+ return scopeObject; >+ } >+ >+ JSStaticScopeObject* clone(ExecState* exec, JSValue init) >+ { >+ JSStaticScopeObject *scopeObject = clone(exec); >+ ASSERT(scopeObject->symbolTable().size() == 1); >+ scopeObject->m_singleRegisterStore.set(exec->globalData(), scopeObject, init); >+ return scopeObject; >+ } >+ >+ int addVariable(const Identifier& name, unsigned attributes) >+ { >+ ASSERT(!m_registers); >+ int idx = symbolTable().size(); >+ symbolTable().add(name.impl(), SymbolTableEntry(idx, attributes)); >+ ASSERT(symbolTable().size() == idx + 1); >+ return idx; >+ } >+ > static void visitChildren(JSCell*, SlotVisitor&); > bool isDynamicScope(bool& requiresDynamicChecks) const; > static JSObject* toThisObject(JSCell*, ExecState*); >@@ -54,24 +98,19 @@ namespace JSC{ > static const ClassInfo s_info; > > protected: >- void finishCreation(ExecState* exec, const Identifier& identifier, JSValue value, unsigned attributes) >- { >- Base::finishCreation(exec->globalData()); >- m_registerStore.set(exec->globalData(), this, value); >- symbolTable().add(identifier.impl(), SymbolTableEntry(-1, attributes)); >- } > static const unsigned StructureFlags = IsEnvironmentRecord | OverridesGetOwnPropertySlot | OverridesVisitChildren | OverridesGetPropertyNames | JSVariableObject::StructureFlags; > > private: >- JSStaticScopeObject(ExecState* exec) >- : JSVariableObject(exec->globalData(), exec->globalData().staticScopeStructure.get(), &m_symbolTable, reinterpret_cast<Register*>(&m_registerStore + 1)) >+ JSStaticScopeObject(ExecState* exec, PassRefPtr<SharedSymbolTable> table) >+ : JSVariableObject(exec->globalData(), exec->globalData().staticScopeStructure.get(), table.get(), 0) >+ , m_symbolTable(table) > { > } > > static void destroy(JSCell*); > >- SymbolTable m_symbolTable; >- WriteBarrier<Unknown> m_registerStore; >+ RefPtr<SharedSymbolTable> m_symbolTable; >+ WriteBarrier<Unknown> m_singleRegisterStore; // For the common case in which only one value is being bound. > }; > > inline bool JSStaticScopeObject::isDynamicScope(bool&) const
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=122959">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Flags:
barraclough
:
review-
Actions:
View
|
Formatted Diff
|
Diff
Attachments on
bug 74633
:
119482
| 122959