From 04ac768f2ac5e3800893370c13c261a075a723ff Mon Sep 17 00:00:00 2001 From: Olivier Goffart Date: Thu, 13 Aug 2009 10:39:28 +0200 Subject: Lazily construct the QScriptActivationObject We can store flags on the ReturnValueRegister entry in the stackframe header (as native function don't use that) Then when requesting an activation object we can lookup the flags to know if we should create it. This reduce a lot the cost of a native call. Reviewed-by: Kent Hansen --- src/script/api/qscriptcontext.cpp | 59 ++++++++++++++++---------- src/script/api/qscriptengine.cpp | 54 +++++++++++++++++------ src/script/api/qscriptengine_p.h | 9 ++++ src/script/api/qscriptvalue.cpp | 5 +++ src/script/bridge/qscriptactivationobject_p.h | 5 +-- tests/auto/qscriptengine/tst_qscriptengine.cpp | 17 +++++++- 6 files changed, 109 insertions(+), 40 deletions(-) diff --git a/src/script/api/qscriptcontext.cpp b/src/script/api/qscriptcontext.cpp index 2d85450..f397ab5 100644 --- a/src/script/api/qscriptcontext.cpp +++ b/src/script/api/qscriptcontext.cpp @@ -329,18 +329,10 @@ bool QScriptContext::isCalledAsConstructor() const { JSC::CallFrame *frame = const_cast(QScriptEnginePrivate::frameForContext(this)); - //For native functions, look up for the QScriptActivationObject and its calledAsConstructor flag. - JSC::ScopeChainNode *node = frame->scopeChain(); - JSC::ScopeChainIterator it(node); - for (it = node->begin(); it != node->end(); ++it) { - if ((*it) && (*it)->isVariableObject()) { - if ((*it)->inherits(&QScript::QScriptActivationObject::info)) { - return static_cast(*it)->d_ptr()->calledAsConstructor; - } - //not a native function - break; - } - } + //For native functions, look up flags. + uint flags = QScriptEnginePrivate::contextFlags(frame); + if (flags & QScriptEnginePrivate::NativeContext) + return flags & QScriptEnginePrivate::CalledAsConstructorContext; //Not a native function, try to look up in the bytecode if we where called from op_construct JSC::Instruction* returnPC = frame->returnPC(); @@ -422,17 +414,21 @@ void QScriptContext::setReturnValue(const QScriptValue &result) \sa argument(), argumentsObject() */ + QScriptValue QScriptContext::activationObject() const { JSC::CallFrame *frame = const_cast(QScriptEnginePrivate::frameForContext(this)); - // ### this is still a bit shaky - // if properties of the activation are accessed after this context is - // popped, we CRASH. - // Ideally we should be able to store the activation object in the callframe - // and JSC would clean it up for us. JSC::JSObject *result = 0; - // look in scope chain - { + + uint flags = QScriptEnginePrivate::contextFlags(frame); + if ((flags & QScriptEnginePrivate::NativeContext) && !(flags & QScriptEnginePrivate::HasScopeContext)) { + //For native functions, lazily create it if needed + QScript::QScriptActivationObject *scope = new (frame) QScript::QScriptActivationObject(frame); + frame->setScopeChain(frame->scopeChain()->copy()->push(scope)); + result = scope; + QScriptEnginePrivate::setContextFlags(frame, flags | QScriptEnginePrivate::HasScopeContext); + } else { + // look in scope chain JSC::ScopeChainNode *node = frame->scopeChain(); JSC::ScopeChainIterator it(node); for (it = node->begin(); it != node->end(); ++it) { @@ -443,14 +439,21 @@ QScriptValue QScriptContext::activationObject() const } } if (!result) { - JSC::CodeBlock *codeBlock = frame->codeBlock(); + if (!parentContext()) + return engine()->globalObject(); + + qWarning("QScriptContext::activationObject: could not get activation object for frame"); + return QScriptValue(); + /*JSC::CodeBlock *codeBlock = frame->codeBlock(); if (!codeBlock) { - // native function + // non-Qt native function + Q_ASSERT(true); //### this should in theorry not happen result = new (frame)QScript::QScriptActivationObject(frame); } else { + // ### this is wrong JSC::FunctionBodyNode *body = static_cast(codeBlock->ownerNode()); result = new (frame)JSC::JSActivation(frame, body); - } + }*/ } return QScript::scriptEngineFromExec(frame)->scriptValueFromJSCValue(result); } @@ -481,7 +484,15 @@ void QScriptContext::setActivationObject(const QScriptValue &activation) return; } - // replace the first activation object in the scope chain + uint flags = QScriptEnginePrivate::contextFlags(frame); + if ((flags & QScriptEnginePrivate::NativeContext) && !(flags & QScriptEnginePrivate::HasScopeContext)) { + //For native functions, we create a scope node + frame->setScopeChain(frame->scopeChain()->copy()->push(object)); + QScriptEnginePrivate::setContextFlags(frame, flags | QScriptEnginePrivate::HasScopeContext); + return; + } + + // else replace the first activation object in the scope chain JSC::ScopeChainNode *node = frame->scopeChain(); while (node != 0) { if (node->object && node->object->isVariableObject()) { @@ -630,6 +641,7 @@ QString QScriptContext::toString() const */ QScriptValueList QScriptContext::scopeChain() const { + activationObject(); //ensure the creation of the normal scope for native context const JSC::CallFrame *frame = QScriptEnginePrivate::frameForContext(this); QScriptEnginePrivate *engine = QScript::scriptEngineFromExec(frame); QScriptValueList result; @@ -652,6 +664,7 @@ QScriptValueList QScriptContext::scopeChain() const */ void QScriptContext::pushScope(const QScriptValue &object) { + activationObject(); //ensure the creation of the normal scope for native context if (!object.isObject()) return; else if (object.engine() != engine()) { diff --git a/src/script/api/qscriptengine.cpp b/src/script/api/qscriptengine.cpp index e8ea222..daf97d5 100644 --- a/src/script/api/qscriptengine.cpp +++ b/src/script/api/qscriptengine.cpp @@ -1114,6 +1114,30 @@ JSC::JSValue QScriptEnginePrivate::thisForContext(JSC::ExecState *frame) } } +/*! \internal + For native context, we use the ReturnValueRegister entry in the stackframe header to store flags. + We can do that because this header is not used as the native function return their value thought C++ + + when setting flags, NativeContext should always be set + + contextFlags returns 0 for non native context + */ +uint QScriptEnginePrivate::contextFlags(JSC::ExecState *exec) +{ + if (exec->codeBlock()) + return 0; //js function doesn't have flags + + return exec->returnValueRegister(); +} + +void QScriptEnginePrivate::setContextFlags(JSC::ExecState *exec, uint flags) +{ + Q_ASSERT(!exec->codeBlock()); + quintptr flag_ptr = flags; + exec->registers()[JSC::RegisterFile::ReturnValueRegister] = JSC::JSValue(reinterpret_cast(flag_ptr)); +} + + void QScriptEnginePrivate::mark() { if (!originalGlobalObject()->marked()) @@ -2126,6 +2150,7 @@ QScriptValue QScriptEngine::evaluate(const QString &program, const QString &file Q_D(QScriptEngine); JSC::JSLock lock(false); // ### hmmm + currentContext()->activationObject(); //force the creation of a context for native function; JSC::UString jscProgram = QScript::qtStringToJSCUString(program); JSC::UString jscFileName = QScript::qtStringToJSCUString(fileName); @@ -2269,6 +2294,10 @@ JSC::CallFrame *QScriptEnginePrivate::pushContext(JSC::CallFrame *exec, const JS thisObject = new (exec) QScriptObject(structure); } + int flags = NativeContext; + if (calledAsConstructor) + flags |= CalledAsConstructorContext; + JSC::CallFrame *newCallFrame = exec; if (callee == 0 || !(exec->callee() == callee && exec->returnPC() != 0)) { //We need to check if the Interpreter might have already created a frame for function called from JS. @@ -2285,16 +2314,16 @@ JSC::CallFrame *QScriptEnginePrivate::pushContext(JSC::CallFrame *exec, const JS for (it = args.begin(); it != args.end(); ++it) newCallFrame[++dst] = *it; newCallFrame += argc + JSC::RegisterFile::CallFrameHeaderSize; - newCallFrame->init(0, /*vPC=*/0, exec->scopeChain(), exec, 0, argc, callee); - } else if (calledAsConstructor) { - //update the new created this - JSC::Register* thisRegister = newCallFrame->registers() - JSC::RegisterFile::CallFrameHeaderSize - newCallFrame->argumentCount(); - *thisRegister = thisObject; + newCallFrame->init(0, /*vPC=*/0, exec->scopeChain(), exec, flags, argc, callee); + } else { + setContextFlags(newCallFrame, flags); + if (calledAsConstructor) { + //update the new created this + JSC::Register* thisRegister = newCallFrame->registers() - JSC::RegisterFile::CallFrameHeaderSize - newCallFrame->argumentCount(); + *thisRegister = thisObject; + } } currentFrame = newCallFrame; - QScript::QScriptActivationObject *scope = new (newCallFrame) QScript::QScriptActivationObject(newCallFrame); - scope->d_ptr()->calledAsConstructor = calledAsConstructor; - newCallFrame->setScopeChain(newCallFrame->scopeChain()->copy()->push(scope)); return newCallFrame; } @@ -2311,7 +2340,7 @@ void QScriptEngine::popContext() agent()->contextPop(); Q_D(QScriptEngine); if (d->currentFrame->returnPC() != 0 || d->currentFrame->codeBlock() != 0 - || d->currentFrame->returnValueRegister() != 0 || !currentContext()->parentContext()) { + || !currentContext()->parentContext()) { qWarning("QScriptEngine::popContext() doesn't match with pushContext()"); return; } @@ -2324,17 +2353,18 @@ void QScriptEngine::popContext() */ void QScriptEnginePrivate::popContext() { + bool hasScope = contextFlags(currentFrame) & HasScopeContext; if (currentFrame->returnPC() == 0) { //normal case JSC::RegisterFile ®isterFile = currentFrame->interpreter()->registerFile(); JSC::Register *const newEnd = currentFrame->registers() - JSC::RegisterFile::CallFrameHeaderSize - currentFrame->argumentCount(); - currentFrame->scopeChain()->pop()->deref(); + if (hasScope) + currentFrame->scopeChain()->pop()->deref(); currentFrame = currentFrame->callerFrame(); registerFile.shrink(newEnd); - } else { //the stack frame was created by the Interpreter, we don't need to rewind it. + } else if(hasScope) { //the stack frame was created by the Interpreter, we don't need to rewind it. currentFrame->setScopeChain(currentFrame->scopeChain()->pop()); currentFrame->scopeChain()->deref(); } - } /*! diff --git a/src/script/api/qscriptengine_p.h b/src/script/api/qscriptengine_p.h index 86c7616..1f59d36 100644 --- a/src/script/api/qscriptengine_p.h +++ b/src/script/api/qscriptengine_p.h @@ -165,6 +165,15 @@ public: bool isCollecting() const; void collectGarbage(); + //flags that we set on the return value register for native function. (ie when codeBlock is 0) + enum ContextFlags { + NativeContext = 1, + CalledAsConstructorContext = 2, + HasScopeContext = 4 + }; + static uint contextFlags(JSC::ExecState *); + static void setContextFlags(JSC::ExecState *, uint); + QScript::TimeoutCheckerProxy *timeoutChecker() const; #ifndef QT_NO_QOBJECT diff --git a/src/script/api/qscriptvalue.cpp b/src/script/api/qscriptvalue.cpp index b02802c..4558097 100644 --- a/src/script/api/qscriptvalue.cpp +++ b/src/script/api/qscriptvalue.cpp @@ -2020,6 +2020,7 @@ QScriptValue QScriptValue::call(const QScriptValue &thisObject, "a different engine"); return QScriptValue(); } + engine()->currentContext()->activationObject(); //force the creation of a context for native function; QScriptEnginePrivate *eng_p = QScriptEnginePrivate::get(d->engine); JSC::ExecState *exec = eng_p->currentFrame; @@ -2095,6 +2096,7 @@ QScriptValue QScriptValue::call(const QScriptValue &thisObject, "a different engine"); return QScriptValue(); } + engine()->currentContext()->activationObject(); //force the creation of a context for native function; QScriptEnginePrivate *eng_p = QScriptEnginePrivate::get(d->engine); JSC::ExecState *exec = eng_p->currentFrame; @@ -2161,6 +2163,7 @@ QScriptValue QScriptValue::construct(const QScriptValueList &args) Q_D(const QScriptValue); if (!isFunction()) return QScriptValue(); + engine()->currentContext()->activationObject(); //force the creation of a context for native function; QScriptEnginePrivate *eng_p = QScriptEnginePrivate::get(d->engine); JSC::ExecState *exec = eng_p->currentFrame; @@ -2172,6 +2175,7 @@ QScriptValue QScriptValue::construct(const QScriptValueList &args) else argsVector[i] = eng_p->scriptValueToJSCValue(args.at(i)); } + JSC::ArgList jscArgs(argsVector.data(), argsVector.size()); JSC::JSValue callee = d->jscValue; @@ -2208,6 +2212,7 @@ QScriptValue QScriptValue::construct(const QScriptValue &arguments) Q_D(QScriptValue); if (!isFunction()) return QScriptValue(); + engine()->currentContext()->activationObject(); //force the creation of a context for native function; QScriptEnginePrivate *eng_p = QScriptEnginePrivate::get(d->engine); JSC::ExecState *exec = eng_p->currentFrame; diff --git a/src/script/bridge/qscriptactivationobject_p.h b/src/script/bridge/qscriptactivationobject_p.h index eb60ae5..8c62b23 100644 --- a/src/script/bridge/qscriptactivationobject_p.h +++ b/src/script/bridge/qscriptactivationobject_p.h @@ -74,12 +74,9 @@ public: struct QScriptActivationObjectData : public JSVariableObjectData { QScriptActivationObjectData(JSC::Register* registers) - : JSVariableObjectData(&symbolTable, registers), calledAsConstructor(false) + : JSVariableObjectData(&symbolTable, registers) { } JSC::SymbolTable symbolTable; - - //specifies if the context of this activation object is called as constructor - bool calledAsConstructor; }; QScriptActivationObjectData *d_ptr() const { return static_cast(d); } diff --git a/tests/auto/qscriptengine/tst_qscriptengine.cpp b/tests/auto/qscriptengine/tst_qscriptengine.cpp index f15ebdf..821bc0c 100644 --- a/tests/auto/qscriptengine/tst_qscriptengine.cpp +++ b/tests/auto/qscriptengine/tst_qscriptengine.cpp @@ -806,10 +806,26 @@ void tst_QScriptEngine::newQMetaObject() QVERIFY(instance3.instanceOf(qclass)); args.clear(); + QPointer qpointer1 = instance.toQObject(); + QPointer qpointer2 = instance2.toQObject(); + QPointer qpointer3 = instance3.toQObject(); + + QVERIFY(qpointer1); + QVERIFY(qpointer2); + QVERIFY(qpointer3); + // verify that AutoOwnership is in effect instance = QScriptValue(); eng.collectGarbage(); + + QEXPECT_FAIL("","collectGarbage not working", Continue); + QVERIFY(!qpointer1); + QVERIFY(qpointer2); + QEXPECT_FAIL("","collectGarbage not working", Continue); + QVERIFY(!qpointer3); // was child of instance + QVERIFY(instance.toQObject() == 0); + QEXPECT_FAIL("","collectGarbage not working", Continue); QVERIFY(instance3.toQObject() == 0); // was child of instance QVERIFY(instance2.toQObject() != 0); instance2 = QScriptValue(); @@ -2060,7 +2076,6 @@ void tst_QScriptEngine::collectGarbage() QScriptValue v = eng.newQObject(ptr, QScriptEngine::ScriptOwnership); } eng.collectGarbage(); - QEXPECT_FAIL("", "", Continue); QVERIFY(ptr == 0); } -- cgit v0.12