Cleanup: Avoid unnecessary irModule->functions.indexOf calls

With a growing number of functions per module, these calls become
expensive and are unnecessary. defineFunction in the code generator
can simply return the correct index right away.

Change-Id: I8ab56a3083bf215674a1b46c502b415be694e465
Reviewed-by: Lars Knoll <lars.knoll@digia.com>

7 files changed, 58 insertions, 50 deletions

diff --git a/src/qml/compiler/qqmlcodegenerator.cpp b/src/qml/compiler/qqmlcodegenerator.cpp
index 41aa17fff3..ed5f4c3772 100644
--- a/src/qml/compiler/qqmlcodegenerator.cpp
+++ b/src/qml/compiler/qqmlcodegenerator.cpp
@@ -1180,7 +1180,7 @@ QVector<int> JSCodeGen::generateJSCodeForFunctionsAndBindings(const QString &fil
     scan.end();
 
     _env = 0;
-    _function = defineFunction(QString("context scope"), qmlRoot, 0, 0);
+    _function = _module->functions.at(defineFunction(QString("context scope"), qmlRoot, 0, 0));
 
     for (int i = 0; i < functions.count(); ++i) {
         AST::Node *node = functions.at(i);
@@ -1212,10 +1212,10 @@ QVector<int> JSCodeGen::generateJSCodeForFunctionsAndBindings(const QString &fil
             body = body->finish();
         }
 
-        V4IR::Function *irFunc = defineFunction(name, node,
-                                                function ? function->formals : 0,
-                                                body);
-        runtimeFunctionIndices[i] = _module->functions.indexOf(irFunc); // ###
+        int idx = defineFunction(name, node,
+                                 function ? function->formals : 0,
+                                 body);
+        runtimeFunctionIndices[i] = idx;
     }
 
     qDeleteAll(_envMap);
diff --git a/src/qml/compiler/qv4codegen.cpp b/src/qml/compiler/qv4codegen.cpp
index 727d20d045..a68920235e 100644
--- a/src/qml/compiler/qv4codegen.cpp
+++ b/src/qml/compiler/qv4codegen.cpp
@@ -413,7 +413,7 @@ Codegen::Codegen(bool strict)
 {
 }
 
-V4IR::Function *Codegen::generateFromProgram(const QString &fileName,
+void Codegen::generateFromProgram(const QString &fileName,
                                   const QString &sourceCode,
                                   Program *node,
                                   V4IR::Module *module,
@@ -430,18 +430,15 @@ V4IR::Function *Codegen::generateFromProgram(const QString &fileName,
     ScanFunctions scan(this, sourceCode, mode);
     scan(node);
 
-    V4IR::Function *globalCode = defineFunction(QStringLiteral("%entry"), node, 0,
-                                              node->elements, inheritedLocals);
+    defineFunction(QStringLiteral("%entry"), node, 0, node->elements, inheritedLocals);
     qDeleteAll(_envMap);
     _envMap.clear();
-
-    return globalCode;
 }
 
-V4IR::Function *Codegen::generateFromFunctionExpression(const QString &fileName,
-                                  const QString &sourceCode,
-                                  AST::FunctionExpression *ast,
-                                  V4IR::Module *module)
+void Codegen::generateFromFunctionExpression(const QString &fileName,
+                                             const QString &sourceCode,
+                                             AST::FunctionExpression *ast,
+                                             V4IR::Module *module)
 {
     _module = module;
     _module->setFileName(fileName);
@@ -453,12 +450,10 @@ V4IR::Function *Codegen::generateFromFunctionExpression(const QString &fileName,
     scan(ast);
     scan.leaveEnvironment();
 
-    V4IR::Function *function = defineFunction(ast->name.toString(), ast, ast->formals, ast->body ? ast->body->elements : 0);
+    defineFunction(ast->name.toString(), ast, ast->formals, ast->body ? ast->body->elements : 0);
 
     qDeleteAll(_envMap);
     _envMap.clear();
-
-    return function;
 }
 
 
@@ -1339,7 +1334,7 @@ bool Codegen::visit(FieldMemberExpression *ast)
 
 bool Codegen::visit(FunctionExpression *ast)
 {
-    V4IR::Function *function = defineFunction(ast->name.toString(), ast, ast->formals, ast->body ? ast->body->elements : 0);
+    int function = defineFunction(ast->name.toString(), ast, ast->formals, ast->body ? ast->body->elements : 0);
     _expr.code = _block->CLOSURE(function);
     return false;
 }
@@ -1460,9 +1455,18 @@ bool Codegen::visit(NumericLiteral *ast)
 }
 
 struct ObjectPropertyValue {
+    ObjectPropertyValue()
+        : value(0)
+        , getter(-1)
+        , setter(-1)
+    {}
+
     V4IR::Expr *value;
-    V4IR::Function *getter;
-    V4IR::Function *setter;
+    int getter; // index in _module->functions or -1 if not set
+    int setter;
+
+    bool hasGetter() const { return getter >= 0; }
+    bool hasSetter() const { return setter >= 0; }
 };
 
 bool Codegen::visit(ObjectLiteral *ast)
@@ -1474,18 +1478,18 @@ bool Codegen::visit(ObjectLiteral *ast)
             QString name = propertyName(nv->name);
             Result value = expression(nv->value);
             ObjectPropertyValue &v = valueMap[name];
-            if (v.getter || v.setter || (_function->isStrict && v.value))
+            if (v.hasGetter() || v.hasSetter() || (_function->isStrict && v.value))
                 throwSyntaxError(nv->lastSourceLocation(),
                                  QCoreApplication::translate("qv4codegen", "Illegal duplicate key '%1' in object literal").arg(name));
 
             valueMap[name].value = *value;
         } else if (PropertyGetterSetter *gs = AST::cast<AST::PropertyGetterSetter *>(it->assignment)) {
             QString name = propertyName(gs->name);
-            V4IR::Function *function = defineFunction(name, gs, gs->formals, gs->functionBody ? gs->functionBody->elements : 0);
+            const int function = defineFunction(name, gs, gs->formals, gs->functionBody ? gs->functionBody->elements : 0);
             ObjectPropertyValue &v = valueMap[name];
             if (v.value ||
-                (gs->type == PropertyGetterSetter::Getter && v.getter) ||
-                (gs->type == PropertyGetterSetter::Setter && v.setter))
+                (gs->type == PropertyGetterSetter::Getter && v.hasGetter()) ||
+                (gs->type == PropertyGetterSetter::Setter && v.hasSetter()))
                 throwSyntaxError(gs->lastSourceLocation(),
                                  QCoreApplication::translate("qv4codegen", "Illegal duplicate key '%1' in object literal").arg(name));
             if (gs->type == PropertyGetterSetter::Getter)
@@ -1535,8 +1539,8 @@ bool Codegen::visit(ObjectLiteral *ast)
 
                 unsigned getter = _block->newTemp();
                 unsigned setter = _block->newTemp();
-                move(_block->TEMP(getter), it->getter ? _block->CLOSURE(it->getter) : _block->CONST(V4IR::UndefinedType, 0));
-                move(_block->TEMP(setter), it->setter ? _block->CLOSURE(it->setter) : _block->CONST(V4IR::UndefinedType, 0));
+                move(_block->TEMP(getter), it->hasGetter() ? _block->CLOSURE(it->getter) : _block->CONST(V4IR::UndefinedType, 0));
+                move(_block->TEMP(setter), it->hasSetter() ? _block->CLOSURE(it->setter) : _block->CONST(V4IR::UndefinedType, 0));
 
                 current->next = _function->New<V4IR::ExprList>();
                 current = current->next;
@@ -1581,8 +1585,8 @@ bool Codegen::visit(ObjectLiteral *ast)
                     getter = _block->newTemp();
                     setter = _block->newTemp();
                 }
-                move(_block->TEMP(getter), it->getter ? _block->CLOSURE(it->getter) : _block->CONST(V4IR::UndefinedType, 0));
-                move(_block->TEMP(setter), it->setter ? _block->CLOSURE(it->setter) : _block->CONST(V4IR::UndefinedType, 0));
+                move(_block->TEMP(getter), it->hasGetter() ? _block->CLOSURE(it->getter) : _block->CONST(V4IR::UndefinedType, 0));
+                move(_block->TEMP(setter), it->hasSetter() ? _block->CLOSURE(it->setter) : _block->CONST(V4IR::UndefinedType, 0));
 
 
                 // __qmljs_builtin_define_getter_setter(Value object, String *name, Value getter, Value setter, ExecutionContext *ctx);
@@ -1750,10 +1754,10 @@ bool Codegen::visit(FunctionDeclaration * ast)
     return false;
 }
 
-V4IR::Function *Codegen::defineFunction(const QString &name, AST::Node *ast,
-                                      AST::FormalParameterList *formals,
-                                      AST::SourceElements *body,
-                                      const QStringList &inheritedLocals)
+int Codegen::defineFunction(const QString &name, AST::Node *ast,
+                            AST::FormalParameterList *formals,
+                            AST::SourceElements *body,
+                            const QStringList &inheritedLocals)
 {
     Loop *loop = 0;
     qSwap(_loop, loop);
@@ -1762,6 +1766,7 @@ V4IR::Function *Codegen::defineFunction(const QString &name, AST::Node *ast,
 
     enterEnvironment(ast);
     V4IR::Function *function = _module->newFunction(name, _function);
+    int functionIndex = _module->functions.count() - 1;
 
     V4IR::BasicBlock *entryBlock = function->newBasicBlock(groupStartBlock());
     V4IR::BasicBlock *exitBlock = function->newBasicBlock(groupStartBlock(), V4IR::Function::DontInsertBlock);
@@ -1840,8 +1845,8 @@ V4IR::Function *Codegen::defineFunction(const QString &name, AST::Node *ast,
 
     foreach (const Environment::Member &member, _env->members) {
         if (member.function) {
-            V4IR::Function *function = defineFunction(member.function->name.toString(), member.function, member.function->formals,
-                                                    member.function->body ? member.function->body->elements : 0);
+            const int function = defineFunction(member.function->name.toString(), member.function, member.function->formals,
+                                                member.function->body ? member.function->body->elements : 0);
             if (! _env->parent || _env->compilationMode == QmlBinding) {
                 move(_block->NAME(member.function->name.toString(), member.function->identifierToken.startLine, member.function->identifierToken.startColumn),
                      _block->CLOSURE(function));
@@ -1873,7 +1878,7 @@ V4IR::Function *Codegen::defineFunction(const QString &name, AST::Node *ast,
 
     leaveEnvironment();
 
-    return function;
+    return functionIndex;
 }
 
 bool Codegen::visit(IdentifierPropertyName *ast)
diff --git a/src/qml/compiler/qv4codegen_p.h b/src/qml/compiler/qv4codegen_p.h
index 2da65fae79..66b80c377b 100644
--- a/src/qml/compiler/qv4codegen_p.h
+++ b/src/qml/compiler/qv4codegen_p.h
@@ -75,13 +75,13 @@ public:
         QmlBinding
     };
 
-    V4IR::Function *generateFromProgram(const QString &fileName,
+    void generateFromProgram(const QString &fileName,
                              const QString &sourceCode,
                              AST::Program *ast,
                              V4IR::Module *module,
                              CompilationMode mode = GlobalCode,
                              const QStringList &inheritedLocals = QStringList());
-    V4IR::Function *generateFromFunctionExpression(const QString &fileName,
+    void generateFromFunctionExpression(const QString &fileName,
                              const QString &sourceCode,
                              AST::FunctionExpression *ast,
                              V4IR::Module *module);
@@ -283,10 +283,11 @@ protected:
     void move(V4IR::Expr *target, V4IR::Expr *source, V4IR::AluOp op = V4IR::OpInvalid);
     void cjump(V4IR::Expr *cond, V4IR::BasicBlock *iftrue, V4IR::BasicBlock *iffalse);
 
-    V4IR::Function *defineFunction(const QString &name, AST::Node *ast,
-                                 AST::FormalParameterList *formals,
-                                 AST::SourceElements *body,
-                                 const QStringList &inheritedLocals = QStringList());
+    // Returns index in _module->functions
+    int defineFunction(const QString &name, AST::Node *ast,
+                       AST::FormalParameterList *formals,
+                       AST::SourceElements *body,
+                       const QStringList &inheritedLocals = QStringList());
 
     void unwindException(ScopeAndFinally *outest);
 
diff --git a/src/qml/compiler/qv4isel_masm.cpp b/src/qml/compiler/qv4isel_masm.cpp
index 888d11a6c5..d5e67d91c3 100644
--- a/src/qml/compiler/qv4isel_masm.cpp
+++ b/src/qml/compiler/qv4isel_masm.cpp
@@ -1071,7 +1071,7 @@ void InstructionSelection::setActivationProperty(V4IR::Expr *source, const QStri
 
 void InstructionSelection::initClosure(V4IR::Closure *closure, V4IR::Temp *target)
 {
-    int id = irModule->functions.indexOf(closure->value);
+    int id = closure->value;
     generateFunctionCall(target, __qmljs_init_closure, Assembler::ContextRegister, Assembler::TrustedImm32(id));
 }
 
diff --git a/src/qml/compiler/qv4isel_moth.cpp b/src/qml/compiler/qv4isel_moth.cpp
index 0b9473b1d3..c610ca6f02 100644
--- a/src/qml/compiler/qv4isel_moth.cpp
+++ b/src/qml/compiler/qv4isel_moth.cpp
@@ -449,7 +449,7 @@ void InstructionSelection::setActivationProperty(V4IR::Expr *source, const QStri
 
 void InstructionSelection::initClosure(V4IR::Closure *closure, V4IR::Temp *target)
 {
-    int id = irModule->functions.indexOf(closure->value);
+    int id = closure->value;
     Instruction::LoadClosure load;
     load.value = id;
     load.result = getResultParam(target);
diff --git a/src/qml/compiler/qv4jsir.cpp b/src/qml/compiler/qv4jsir.cpp
index 756c076abf..869bf4acaf 100644
--- a/src/qml/compiler/qv4jsir.cpp
+++ b/src/qml/compiler/qv4jsir.cpp
@@ -464,7 +464,7 @@ bool operator<(const Temp &t1, const Temp &t2) Q_DECL_NOTHROW
 
 void Closure::dump(QTextStream &out) const
 {
-    QString name = value->name ? *value->name : QString();
+    QString name = functionName ? *functionName : QString();
     if (name.isEmpty())
         name.sprintf("%p", value);
     out << "closure(" << name << ')';
@@ -763,10 +763,10 @@ Name *BasicBlock::NAME(Name::Builtin builtin, quint32 line, quint32 column)
     return e;
 }
 
-Closure *BasicBlock::CLOSURE(Function *function)
+Closure *BasicBlock::CLOSURE(int functionInModule)
 {
     Closure *clos = function->New<Closure>();
-    clos->init(function);
+    clos->init(functionInModule, function->module->functions.at(functionInModule)->name);
     return clos;
 }
 
diff --git a/src/qml/compiler/qv4jsir_p.h b/src/qml/compiler/qv4jsir_p.h
index f28c47a61a..d0782a3a43 100644
--- a/src/qml/compiler/qv4jsir_p.h
+++ b/src/qml/compiler/qv4jsir_p.h
@@ -386,11 +386,13 @@ inline uint qHash(const Temp &t, uint seed = 0) Q_DECL_NOTHROW
 bool operator<(const Temp &t1, const Temp &t2) Q_DECL_NOTHROW;
 
 struct Closure: Expr {
-    Function *value;
+    int value; // index in _module->functions
+    const QString *functionName;
 
-    void init(Function *value)
+    void init(int functionInModule, const QString *functionName)
     {
-        this->value = value;
+        this->value = functionInModule;
+        this->functionName = functionName;
     }
 
     virtual void accept(ExprVisitor *v) { v->visitClosure(this); }
@@ -812,7 +814,7 @@ struct BasicBlock {
 
     Name *GLOBALNAME(const QString &id, quint32 line, quint32 column);
 
-    Closure *CLOSURE(Function *function);
+    Closure *CLOSURE(int functionInModule);
 
     Expr *CONVERT(Expr *expr, Type type);
     Expr *UNOP(AluOp op, Expr *expr);