Optimize Add/Remove SG::RenderTask from SG::RenderTaskList

Let we make render task list use OrderedSet instead of vector container.

Vector container is slow if we call RemoveRenderTask.

TODO : Optimize event side render task list too.
But since it is quite complex, we'd better optimize that points
in another patch

Change-Id: Icd6f2e7a24964eb05c92fa9a5ce0e306814445ee
Signed-off-by: Eunki Hong <eunkiki.hong@samsung.com>

diff --git a/automated-tests/src/dali-internal/utc-Dali-Internal-OrderedSet.cpp b/automated-tests/src/dali-internal/utc-Dali-Internal-OrderedSet.cpp
index abc2cb62f8399dc4a2d55740758b6abc88652634..6f98cb95bca627111d7d7735ae409a4e221034e2 100644 (file)
--- a/automated-tests/src/dali-internal/utc-Dali-Internal-OrderedSet.cpp
+++ b/automated-tests/src/dali-internal/utc-Dali-Internal-OrderedSet.cpp
@@ -471,5 +471,89 @@ int UtcDaliOrderedSetFalseIteratorOrderCheck(void)
   objectList.clear();
   DALI_TEST_EQUALS(ClassWithId::gRefCount, 0, TEST_LOCATION);
 
+  END_TEST;
+}
+
+int UtcDaliOrderedSetReorderCacheMap(void)
+{
+  // Ensure that order of iterator is equal with Order of Insertion.
+  // And also check the order is valid even if change the order by user, after we call ReorderCacheMap.
+
+  // Reset refcount of class
+  ClassWithId::gRefCount = 0;
+
+  // To avoid lucky pass, run this test multiple time.
+  int tryCnt = 3;
+  while(tryCnt--)
+  {
+    int baseId = tryCnt; // random id
+    int id     = baseId;
+    int n      = 10 + 5 * (tryCnt + 1); // random count
+
+    OrderedSet<ClassWithId> set;
+
+    for(int i = 0; i < n; ++i)
+    {
+      ClassWithId* object = new ClassWithId(id++);
+      set.PushBack(object);
+    }
+
+    int expectId;
+    // Check by for iteration
+    expectId = baseId;
+    for(auto iter = set.Begin(), iterEnd = set.End(); iter != iterEnd; ++iter)
+    {
+      DALI_TEST_EQUALS(expectId++, (*iter)->mId, TEST_LOCATION);
+    }
+    DALI_TEST_EQUALS(expectId, id, TEST_LOCATION);
+
+    // Shuffle it randomly.
+    std::vector<std::pair<int, ClassWithId*>> shuffleList;
+    shuffleList.reserve(n);
+    for(auto iter = set.Begin(), iterEnd = set.End(); iter != iterEnd; ++iter)
+    {
+      shuffleList.emplace_back((*iter)->mId, (*iter));
+    }
+    std::random_shuffle(shuffleList.begin(), shuffleList.end());
+
+    // Change the value of container as shuffled order. After then, call ReorderCacheMap().
+    int shuffleIndex = 0;
+    for(auto iter = set.Begin(), iterEnd = set.End(); iter != iterEnd; ++iter)
+    {
+      (*iter) = shuffleList[shuffleIndex++].second;
+    }
+    set.ReorderCacheMap();
+
+    // Check by iterator. And randomly erase item
+    shuffleIndex = 0;
+    for(auto iter = set.Begin(), iterEnd = set.End(); iter != iterEnd; ++iter)
+    {
+      DALI_TEST_EQUALS(shuffleList[shuffleIndex++].first, (*iter)->mId, TEST_LOCATION);
+    }
+
+    // Randomly erase item, and check again until all items removed.
+    while(!set.IsEmpty())
+    {
+      int removeIndex = rand()%set.Count();
+      auto iter = set.Find(shuffleList[removeIndex].second);
+      DALI_TEST_CHECK(iter != set.End());
+      set.Erase(iter);
+      for(int i = removeIndex + 1; i < n; ++i)
+      {
+        shuffleList[i - 1] = shuffleList[i];
+      }
+      --n;
+
+      shuffleIndex = 0;
+      for(auto iter = set.Begin(), iterEnd = set.End(); iter != iterEnd; ++iter)
+      {
+        DALI_TEST_EQUALS(shuffleList[shuffleIndex++].first, (*iter)->mId, TEST_LOCATION);
+      }
+    }
+  }
+
+  // Check whether leak exist.
+  DALI_TEST_EQUALS(ClassWithId::gRefCount, 0, TEST_LOCATION);
+
   END_TEST;
 }
\ No newline at end of file

diff --git a/dali/integration-api/ordered-set.h b/dali/integration-api/ordered-set.h
index a511805ad93b4528b45430d07e47767946a015a4..b235691a453193f74f62b70a7ab8439247a75494 100644 (file)
--- a/dali/integration-api/ordered-set.h
+++ b/dali/integration-api/ordered-set.h
@@ -159,6 +159,16 @@ public:
     return mMap.size();
   }
 
+  /**
+   * @brief Predicate to determine if the container is empty
+   *
+   * @return true if the container is empty
+   */
+  bool IsEmpty() const
+  {
+    return mMap.empty();
+  }
+
   /**
    * @brief Reserves space in the ordered set.
    *
@@ -328,6 +338,18 @@ public:
     mList.clear();
   }
 
+  /**
+   * @brief Reorder cache map. It should be called after the value of mList changed.
+   */
+  void ReorderCacheMap()
+  {
+    mMap.clear();
+    for(auto iter = mList.begin(), iterEnd = mList.end(); iter != iterEnd; ++iter)
+    {
+      mMap.insert({*iter, iter});
+    }
+  }
+
 private:
   // Delete copy operation.
   OrderedSet(const OrderedSet&) = delete;

diff --git a/dali/internal/update/render-tasks/scene-graph-render-task-list.cpp b/dali/internal/update/render-tasks/scene-graph-render-task-list.cpp
index 128c8589cb87fa887dd04dae6b93a48fc3a73da1..2957901c85d5a287661e1cc4bb8d08d1c88c9609 100644 (file)
--- a/dali/internal/update/render-tasks/scene-graph-render-task-list.cpp
+++ b/dali/internal/update/render-tasks/scene-graph-render-task-list.cpp
@@ -75,27 +75,28 @@ void RenderTaskList::AddTask(OwnerPointer<RenderTask>& newTask)
 
 void RenderTaskList::RemoveTask(RenderTask* task)
 {
-  RenderTaskContainer::ConstIterator end = mRenderTasks.End();
-  for(RenderTaskContainer::Iterator iter = mRenderTasks.Begin(); iter != end; ++iter)
+  auto iter = mRenderTasks.Find(task);
+  if(iter != mRenderTasks.End())
   {
-    if(*iter == task)
-    {
-      // Destroy the task
-      mRenderTasks.Erase(iter);
-
-      break; // we're finished
-    }
+    // Destroy the task
+    mRenderTasks.Erase(iter);
   }
 }
 
 void RenderTaskList::SortTasks(OwnerPointer<std::vector<const SceneGraph::RenderTask*>>& sortedTasks)
 {
-  for(uint32_t i = 0; i < sortedTasks->size(); ++i)
+  DALI_ASSERT_ALWAYS(sortedTasks->size() == mRenderTasks.Count() && "SceneGraph RenderTask list is not matched with Event side RenderTask list!");
+
+  auto iter = mRenderTasks.Begin();
+  for(auto sortedIter = sortedTasks->begin(), sortedIterEnd = sortedTasks->end(); sortedIter != sortedIterEnd; ++sortedIter, ++iter)
   {
-    const SceneGraph::RenderTask* task = (*sortedTasks)[i];
+    const SceneGraph::RenderTask* task = *sortedIter;
     SceneGraph::RenderTask* castedTask = const_cast<SceneGraph::RenderTask*>(task);
-    mRenderTasks[i]                    = castedTask;
+    (*iter)                            = castedTask;
   }
+
+  // We should call Reorder after the order of container changed.
+  mRenderTasks.ReorderCacheMap();
 }
 
 uint32_t RenderTaskList::GetTaskCount()

diff --git a/dali/internal/update/render-tasks/scene-graph-render-task-list.h b/dali/internal/update/render-tasks/scene-graph-render-task-list.h
index 4df6ec24f5280d2c935955b867304fa03b491a07..68442401e9e46f8732521d94d3317a385b7fd343 100644 (file)
--- a/dali/internal/update/render-tasks/scene-graph-render-task-list.h
+++ b/dali/internal/update/render-tasks/scene-graph-render-task-list.h
@@ -19,7 +19,7 @@
  */
 
 // INTERNAL INCLUDES
-#include <dali/devel-api/common/owner-container.h>
+#include <dali/integration-api/ordered-set.h>
 #include <dali/internal/common/message.h>
 #include <dali/internal/event/common/event-thread-services.h>
 #include <dali/internal/update/render-tasks/scene-graph-render-task.h>
@@ -42,7 +42,7 @@ class ResetterManager;
 class RenderTaskList
 {
 public:
-  using RenderTaskContainer = OwnerContainer<RenderTask*>;
+  using RenderTaskContainer = Dali::Integration::OrderedSet<RenderTask>;
 
   /**
    * Construct a new RenderTaskList.