Avoid useless copy of arrays in ForEach event generated code

Lists of objects were wrongly initialized, creating a useless performance hit. All the objects of the "for each" were copied into arrays, before being emptied. And this, for every single object in the for each loop. This was particularly bad when a lot of objects were picked by the for each (if we have N objects, complexity was N*N copies of the array).

Core/GDCore/Events/CodeGeneration/EventsCodeGenerationContext.cpp

@@ -28,6 +28,10 @@ void EventsCodeGenerationContext::InheritsFrom(
             parent_.objectsListsWithoutPickingToBeDeclared.end(),
             std::inserter(alreadyDeclaredObjectsLists,
                           alreadyDeclaredObjectsLists.begin()));
+  std::copy(parent_.emptyObjectsListsToBeDeclared.begin(),
+            parent_.emptyObjectsListsToBeDeclared.end(),
+            std::inserter(alreadyDeclaredObjectsLists,
+                          alreadyDeclaredObjectsLists.begin()));
 
   depthOfLastUse = parent_.depthOfLastUse;
   customConditionDepth = parent_.customConditionDepth;
@@ -47,8 +51,7 @@ void EventsCodeGenerationContext::Reuse(
 
 void EventsCodeGenerationContext::ObjectsListNeeded(
     const gd::String& objectName) {
-  if (objectsListsWithoutPickingToBeDeclared.find(objectName) ==
+  if (!IsToBeDeclared(objectName))
-      objectsListsWithoutPickingToBeDeclared.end())
     objectsListsToBeDeclared.insert(objectName);
 
   depthOfLastUse[objectName] = GetContextDepth();
@@ -56,19 +59,28 @@ void EventsCodeGenerationContext::ObjectsListNeeded(
 
 void EventsCodeGenerationContext::ObjectsListWithoutPickingNeeded(
     const gd::String& objectName) {
-  if (objectsListsToBeDeclared.find(objectName) ==
+  if (!IsToBeDeclared(objectName))
-      objectsListsToBeDeclared.end())
     objectsListsWithoutPickingToBeDeclared.insert(objectName);
 
   depthOfLastUse[objectName] = GetContextDepth();
 }
 
+void EventsCodeGenerationContext::EmptyObjectsListNeeded(
+    const gd::String& objectName) {
+  if (!IsToBeDeclared(objectName))
+    emptyObjectsListsToBeDeclared.insert(objectName);
+
+  depthOfLastUse[objectName] = GetContextDepth();
+}
+
 std::set<gd::String> EventsCodeGenerationContext::GetAllObjectsToBeDeclared()
     const {
   std::set<gd::String> allObjectListsToBeDeclared(
       objectsListsToBeDeclared.begin(), objectsListsToBeDeclared.end());
   allObjectListsToBeDeclared.insert(objectsListsWithoutPickingToBeDeclared.begin(),
                                     objectsListsWithoutPickingToBeDeclared.end());
+  allObjectListsToBeDeclared.insert(emptyObjectsListsToBeDeclared.begin(),
+                                    emptyObjectsListsToBeDeclared.end());
 
   return allObjectListsToBeDeclared;
 }

Core/GDCore/Events/CodeGeneration/EventsCodeGenerationContext.h

@@ -117,12 +117,22 @@ class GD_CORE_API EventsCodeGenerationContext {
    * Call this when an instruction in the event needs an empty objects list
    * or the one already declared, if any.
    *
-   * An empty objects list will be declared, without filling it with objects from
+   * An empty objects list will be declared, without filling it with objects
-   * the scene. If there is already an objects list with this name, no new list
+   * from the scene. If there is already an objects list with this name, no new
-   * will be declared again.
+   * list will be declared again.
    */
   void ObjectsListWithoutPickingNeeded(const gd::String& objectName);
 
+  /**
+   * Call this when an instruction in the event needs an empty object list,
+   * even if one is already declared.
+   *
+   * An empty objects list will be declared, without filling it with objects
+   * from the scene. If there is already an object list with this name, it won't
+   * be used to initialize the new list, which will remain empty.
+   */
+  void EmptyObjectsListNeeded(const gd::String& objectName);
+
   /**
    * Return true if an object list has already been declared (or is going to be
    * declared).
@@ -161,6 +171,15 @@ class GD_CORE_API EventsCodeGenerationContext {
     return objectsListsWithoutPickingToBeDeclared;
   };
 
+  /**
+   * Return the objects lists which will be will be declared empty, without
+   * filling them with objects from the scene and without copying any previously
+   * declared objects list.
+   */
+  const std::set<gd::String>& GetObjectsListsToBeDeclaredEmpty() const {
+    return emptyObjectsListsToBeDeclared;
+  };
+
   /**
    * Return the objects lists which are already declared and can be used in the
    * current context without declaration.
@@ -209,6 +228,21 @@ class GD_CORE_API EventsCodeGenerationContext {
   size_t GetCurrentConditionDepth() const { return customConditionDepth; }
 
  private:
+  /**
+   * \brief Returns true if the given object is already going to be declared
+   * (either as a traditional objects list, or one without picking, or one
+   * empty).
+   *
+   */
+  bool IsToBeDeclared(const gd::String& objectName) {
+    return objectsListsToBeDeclared.find(objectName) !=
+               objectsListsToBeDeclared.end() ||
+           objectsListsWithoutPickingToBeDeclared.find(objectName) !=
+               objectsListsWithoutPickingToBeDeclared.end() ||
+           emptyObjectsListsToBeDeclared.find(objectName) !=
+               emptyObjectsListsToBeDeclared.end();
+  };
+
   std::set<gd::String>
       alreadyDeclaredObjectsLists;  ///< Objects lists already needed in a
                                     ///< parent context.
@@ -220,6 +254,12 @@ class GD_CORE_API EventsCodeGenerationContext {
                                                ///< declared in this context,
                                                ///< but not filled with scene's
                                                ///< objects.
+  std::set<gd::String>
+      emptyObjectsListsToBeDeclared;  ///< Objects lists that will be
+                                      ///< declared in this context,
+                                      ///< but not filled with scene's
+                                      ///< objects and not filled with any
+                                      ///< previously existing objects list.
   std::map<gd::String, unsigned int>
       depthOfLastUse;  ///< The context depth when an object was last used.
   gd::String

Core/GDCore/Events/CodeGeneration/EventsCodeGenerator.cpp

@@ -751,6 +751,18 @@ gd::String EventsCodeGenerator::GenerateObjectsDeclarationCode(
 
     declarationsCode += objectListDeclaration + "\n";
   }
+  for (auto object : context.GetObjectsListsToBeDeclaredEmpty()) {
+    gd::String objectListDeclaration = "";
+    if (!context.ObjectAlreadyDeclared(object)) {
+      objectListDeclaration = "std::vector<RuntimeObject*> " +
+                              GetObjectListName(object, context) + ";\n";
+      context.SetObjectDeclared(object);
+    } else
+      objectListDeclaration = "std::vector<RuntimeObject*> " +
+                              GetObjectListName(object, context) + ";\n";
+
+    declarationsCode += objectListDeclaration + "\n";
+  }
 
   return declarationsCode;
 }

Core/tests/EventsCodeGenerationContext.cpp

@@ -18,13 +18,13 @@
 TEST_CASE("EventsCodeGenerationContext", "[common][events]") {
   /**
    * Generate a tree of contexts with declared objects as below:
-   *                   c1 -> c1.object1, c1.object2, c1.noPicking1 (empty list
+   *                   c1 -> c1.object1, c1.object2, c1.noPicking1 (no picking
    * request)
    *                  /  \
    *  c2.object1 <- c2   c3 -> c3.object1, c1.object2
    *               /  \
-   *              c4  c5 -> c5.object1, c5.noPicking1 (empty list request),
+   *              c4  c5 -> c5.object1, c5.noPicking1 (no picking request),
-   * c1.object2
+   * c1.object2, c5.empty1
    */
 
   unsigned int maxDepth = 0;
@@ -50,6 +50,7 @@ TEST_CASE("EventsCodeGenerationContext", "[common][events]") {
   c5.ObjectsListWithoutPickingNeeded("c5.noPicking1");
   c5.ObjectsListNeeded("c5.object1");
   c5.ObjectsListNeeded("c1.object2");
+  c5.EmptyObjectsListNeeded("c5.empty1");
 
   SECTION("Parenting") {
     REQUIRE(c2.GetParentContext() == &c1);
@@ -100,8 +101,10 @@ TEST_CASE("EventsCodeGenerationContext", "[common][events]") {
             std::set<gd::String>({"c5.object1", "c1.object2"}));
     REQUIRE(c5.GetObjectsListsToBeDeclaredWithoutPicking() ==
             std::set<gd::String>({"c5.noPicking1"}));
+    REQUIRE(c5.GetObjectsListsToBeDeclaredEmpty() ==
+            std::set<gd::String>({"c5.empty1"}));
     REQUIRE(c5.GetAllObjectsToBeDeclared() ==
-            std::set<gd::String>({"c5.object1", "c5.noPicking1", "c1.object2"}));
+            std::set<gd::String>({"c5.object1", "c5.noPicking1", "c1.object2", "c5.empty1"}));
   }
 
   SECTION("ObjectAlreadyDeclared") {
@@ -130,6 +133,7 @@ TEST_CASE("EventsCodeGenerationContext", "[common][events]") {
     REQUIRE(c5.GetLastDepthObjectListWasNeeded("c1.object2") == 2);
     REQUIRE(c5.GetLastDepthObjectListWasNeeded("c2.object1") == 1);
     REQUIRE(c5.GetLastDepthObjectListWasNeeded("c5.object1") == 2);
+    REQUIRE(c5.GetLastDepthObjectListWasNeeded("c5.empty1") == 2);
   }
 
   SECTION("SetCurrentObject") {
@@ -145,12 +149,12 @@ TEST_CASE("EventsCodeGenerationContext", "[common][events]") {
      * Generate a tree of contexts with declared objects as below:
      *                  ...
      *                   \
-     *                   c5 -> c5.object1, c5.noPicking1 (empty list request),
+     *                   c5 -> c5.object1, c5.noPicking1 (no picking request),
      * c1.object2
      *                  /
      *                c6  (reuse c5) -> c5.object1, c6.object3
      *               /
-     *              c7 -> c5.object1
+     *              c7 -> c5.object1, c5.empty1 (empty list request)
      */
     gd::EventsCodeGenerationContext c6;
     c6.Reuse(c5);
@@ -161,6 +165,7 @@ TEST_CASE("EventsCodeGenerationContext", "[common][events]") {
     c7.InheritsFrom(c6);
     c7.ObjectsListNeeded("c5.object1");
     c7.ObjectsListNeeded("c6.object3");
+    c7.EmptyObjectsListNeeded("c5.empty1");
 
     // c6 is reusing c5 context so it has the same depth:
     REQUIRE(c6.GetParentContext() == &c5);
@@ -179,5 +184,6 @@ TEST_CASE("EventsCodeGenerationContext", "[common][events]") {
     REQUIRE(c7.GetContextDepth() == 3);
     REQUIRE(c7.IsSameObjectsList("c5.object1", c6) == false);
     REQUIRE(c7.IsSameObjectsList("c6.object3", c6) == false);
+    REQUIRE(c7.IsSameObjectsList("c5.empty1", c5) == false);
   }
 }

GDJS/GDJS/Events/CodeGeneration/EventsCodeGenerator.cpp

@@ -644,7 +644,7 @@ gd::String EventsCodeGenerator::GenerateObjectsDeclarationCode(
     if (!context.ObjectAlreadyDeclared(object)) {
       objectListDeclaration += GetObjectListName(object, context) +
                                ".createFrom(" +
-                               GenerateAllInstancesGetter(object) + ");";
+                               GenerateAllInstancesGetterCode(object) + ");";
       context.SetObjectDeclared(object);
     } else
       objectListDeclaration = declareObjectList(object, context);
@@ -662,11 +662,23 @@ gd::String EventsCodeGenerator::GenerateObjectsDeclarationCode(
 
     declarationsCode += objectListDeclaration + "\n";
   }
+  for (auto object : context.GetObjectsListsToBeDeclaredEmpty()) {
+    gd::String objectListDeclaration = "";
+    if (!context.ObjectAlreadyDeclared(object)) {
+      objectListDeclaration =
+          GetObjectListName(object, context) + ".length = 0;\n";
+      context.SetObjectDeclared(object);
+    } else
+      objectListDeclaration =
+          GetObjectListName(object, context) + ".length = 0;\n";
+
+    declarationsCode += objectListDeclaration + "\n";
+  }
 
   return declarationsCode;
 }
 
-gd::String EventsCodeGenerator::GenerateAllInstancesGetter(
+gd::String EventsCodeGenerator::GenerateAllInstancesGetterCode(
     gd::String& objectName) {
   if (HasProjectAndLayout()) {
     return "runtimeScene.getObjects(" + ConvertToStringExplicit(objectName) +

GDJS/GDJS/Events/CodeGeneration/EventsCodeGenerator.h

@@ -249,7 +249,7 @@ class EventsCodeGenerator : public gd::EventsCodeGenerator {
   virtual gd::String GenerateObjectsDeclarationCode(
       gd::EventsCodeGenerationContext& context);
 
-  virtual gd::String GenerateAllInstancesGetter(gd::String& objectName);
+  virtual gd::String GenerateAllInstancesGetterCode(gd::String& objectName);
 
   virtual gd::String GenerateProfilerSectionBegin(const gd::String& section);
   virtual gd::String GenerateProfilerSectionEnd(const gd::String& section);

GDJS/GDJS/Extensions/Builtin/CommonInstructionsExtension.cpp

@@ -341,8 +341,8 @@ CommonInstructionsExtension::CommonInstructionsExtension() {
         gd::String outputCode;
         gd::WhileEvent& event = dynamic_cast<gd::WhileEvent&>(event_);
 
-        // Context is "reset" each time the event is repeated ( i.e. objects are
+        // Context is "reset" each time the event is repeated (i.e. objects are
-        // picked again )
+        // picked again)
         gd::EventsCodeGenerationContext context;
         context.InheritsFrom(parentContext);
         context.ForbidReuse();
@@ -414,8 +414,8 @@ CommonInstructionsExtension::CommonInstructionsExtension() {
             gd::ExpressionCodeGenerator::GenerateExpressionCode(
                 codeGenerator, parentContext, "number", repeatNumberExpression);
 
-        // Context is "reset" each time the event is repeated ( i.e. objects are
+        // Context is "reset" each time the event is repeated (i.e. objects are
-        // picked again )
+        // picked again)
         gd::EventsCodeGenerationContext context;
         context.InheritsFrom(parentContext);
         context.ForbidReuse();
@@ -484,14 +484,14 @@ CommonInstructionsExtension::CommonInstructionsExtension() {
         for (unsigned int i = 0; i < realObjects.size(); ++i)
           parentContext.ObjectsListNeeded(realObjects[i]);
 
-        // Context is "reset" each time the event is repeated ( i.e. objects are
+        // Context is "reset" each time the event is repeated (i.e. objects are
-        // picked again )
+        // picked again)
         gd::EventsCodeGenerationContext context;
         context.InheritsFrom(parentContext);
         context.ForbidReuse();
 
         for (unsigned int i = 0; i < realObjects.size(); ++i)
-          context.ObjectsListNeeded(realObjects[i]);
+          context.EmptyObjectsListNeeded(realObjects[i]);
 
         // Prepare conditions/actions codes
         gd::String conditionsCode = codeGenerator.GenerateConditionsListCode(
@@ -569,10 +569,13 @@ CommonInstructionsExtension::CommonInstructionsExtension() {
                         " < " + forEachTotalCountVar + ";++" + forEachIndexVar +
                         ") {\n";
 
+        // Empty object lists declaration
         outputCode += objectDeclaration;
 
-        // Clear all concerned objects lists and keep only one object
+        // Pick one object
         if (realObjects.size() == 1) {
+          // We write a slighty more simple ( and optimized ) output code
+          // when only one object list is used.
           gd::String temporary = codeGenerator.GetCodeNamespaceAccessor() +
                                  "forEachTemporary" +
                                  gd::String::From(context.GetContextDepth());
@@ -581,22 +584,13 @@ CommonInstructionsExtension::CommonInstructionsExtension() {
               temporary + " = " +
               codeGenerator.GetObjectListName(realObjects[0], parentContext) +
               "[" + forEachIndexVar + "];\n";
-          outputCode +=
+
-              codeGenerator.GetObjectListName(realObjects[0], context) +
-              ".length = 0;\n";
           outputCode +=
               codeGenerator.GetObjectListName(realObjects[0], context) +
               ".push(" + temporary + ");\n";
         } else {
-          // Declare all lists of concerned objects empty
+          // Generate the code to pick only one object in the lists
-          for (unsigned int j = 0; j < realObjects.size(); ++j)
+          for (unsigned int i = 0; i < realObjects.size(); ++i) {
-            outputCode +=
-                codeGenerator.GetObjectListName(realObjects[j], context) +
-                ".length = 0;\n";
-
-          for (unsigned int i = 0; i < realObjects.size();
-               ++i)  // Pick then only one object
-          {
             gd::String count;
             for (unsigned int j = 0; j <= i; ++j) {
               gd::String forEachCountVar =