refactor BaseSignal class. 02/244202/5
authorSubhransu Mohanty <sub.mohanty@samsung.com>
Tue, 15 Sep 2020 06:00:03 +0000 (15:00 +0900)
committerSubhransu Mohanty <sub.mohanty@samsung.com>
Wed, 7 Oct 2020 00:37:53 +0000 (09:37 +0900)
- replace Dali::Vector<SignalConnection*> with std::vector<SignalConnection>
- Fixed Empty() and GetConnectionCount() implementaion complexity from O(n) to O(1).
- move trivial functions to header to make them inline.
- use erase-remove idiom to cleanup the empty connections.

Change-Id: I941bcf6b0b27a14d29a316f005f37bfca44c3530

dali/public-api/signals/base-signal.cpp
dali/public-api/signals/base-signal.h

index 0b3cfd7..e36ae74 100644 (file)
@@ -47,42 +47,17 @@ BaseSignal::~BaseSignal()
 
   // The signal is being destroyed. We have to inform any slots
   // that are connected, that the signal is dead.
-  const std::size_t count(mSignalConnections.Count());
+  const std::size_t count(mSignalConnections.size());
   for(std::size_t i = 0; i < count; i++)
   {
-    SignalConnection* connection = mSignalConnections[i];
+    auto& connection = mSignalConnections[i];
 
     // Note that values are set to NULL in DeleteConnection
     if(connection)
     {
-      connection->Disconnect(this);
-      delete connection;
+      connection.Disconnect(this);
     }
   }
-
-  mSignalConnections.Clear();
-}
-
-bool BaseSignal::Empty() const
-{
-  return (0 == GetConnectionCount());
-}
-
-std::size_t BaseSignal::GetConnectionCount() const
-{
-  std::size_t count(0);
-
-  const std::size_t size(mSignalConnections.Count());
-  for(std::size_t i = 0; i < size; ++i)
-  {
-    // Note that values are set to NULL in DeleteConnection
-    if(nullptr != mSignalConnections[i])
-    {
-      ++count;
-    }
-  }
-
-  return count;
 }
 
 void BaseSignal::Emit()
@@ -96,7 +71,7 @@ void BaseSignal::Emit()
 
   // If more connections are added by callbacks, these are ignore until the next Emit()
   // Note that mSignalConnections.Count() count cannot be reduced while iterating
-  const std::size_t initialCount(mSignalConnections.Count());
+  const std::size_t initialCount(mSignalConnections.size());
 
   for(std::size_t i = 0; i < initialCount; ++i)
   {
@@ -124,9 +99,9 @@ void BaseSignal::OnConnect(CallbackBase* callback)
   if(INVALID_CALLBACK_INDEX == index)
   {
     // create a new signal connection object, to allow the signal to track the connection.
-    SignalConnection* connection = new SignalConnection(callback);
+    //SignalConnection* connection = new SignalConnection(callback);
 
-    mSignalConnections.PushBack(connection);
+    mSignalConnections.push_back(SignalConnection(callback));
   }
   else
   {
@@ -161,9 +136,9 @@ void BaseSignal::OnConnect(ConnectionTrackerInterface* tracker, CallbackBase* ca
   if(INVALID_CALLBACK_INDEX == index)
   {
     // create a new signal connection object, to allow the signal to track the connection.
-    SignalConnection* connection = new SignalConnection(tracker, callback);
+    //SignalConnection* connection = new SignalConnection(tracker, callback);
 
-    mSignalConnections.PushBack(connection);
+    mSignalConnections.push_back({tracker, callback});
 
     // Let the connection tracker know that a connection between a signal and a slot has been made.
     tracker->SignalConnected(this, callback);
@@ -185,7 +160,7 @@ void BaseSignal::OnDisconnect(ConnectionTrackerInterface* tracker, CallbackBase*
   if(index > INVALID_CALLBACK_INDEX)
   {
     // temporary pointer to disconnected callback
-    CallbackBase* disconnectedCallback = mSignalConnections[index]->GetCallback();
+    CallbackBase* disconnectedCallback = mSignalConnections[index].GetCallback();
 
     // close the signal side connection first.
     DeleteConnection(index);
@@ -201,7 +176,7 @@ void BaseSignal::OnDisconnect(ConnectionTrackerInterface* tracker, CallbackBase*
 // for SlotObserver::SlotDisconnected
 void BaseSignal::SlotDisconnected(CallbackBase* callback)
 {
-  const std::size_t count(mSignalConnections.Count());
+  const std::size_t count(mSignalConnections.size());
   for(std::size_t i = 0; i < count; ++i)
   {
     const CallbackBase* connectionCallback = GetCallback(i);
@@ -220,32 +195,15 @@ void BaseSignal::SlotDisconnected(CallbackBase* callback)
   DALI_ABORT("Callback lost in SlotDisconnected()");
 }
 
-CallbackBase* BaseSignal::GetCallback(std::size_t connectionIndex) const
-{
-  DALI_ASSERT_ALWAYS(connectionIndex < mSignalConnections.Count() && "GetCallback called with invalid index");
-
-  CallbackBase* callback(nullptr);
-
-  SignalConnection* connection(mSignalConnections[connectionIndex]);
-
-  // Note that values are set to NULL in DeleteConnection
-  if(connection)
-  {
-    callback = connection->GetCallback();
-  }
 
-  return callback;
-}
-
-int32_t BaseSignal::FindCallback(CallbackBase* callback)
+int32_t BaseSignal::FindCallback(CallbackBase* callback) const noexcept
 {
   int32_t index(INVALID_CALLBACK_INDEX);
 
   // A signal can have multiple slots connected to it.
   // We need to search for the slot which has the same call back function (if it's static)
   // Or the same object / member function (for non-static)
-  const std::size_t count = mSignalConnections.Count();
-  for(std::size_t i = 0; i < count; ++i)
+  for(auto i = 0u; i < mSignalConnections.size(); ++i)
   {
     const CallbackBase* connectionCallback = GetCallback(i);
 
@@ -262,49 +220,34 @@ int32_t BaseSignal::FindCallback(CallbackBase* callback)
 
 void BaseSignal::DeleteConnection(std::size_t connectionIndex)
 {
-  DALI_ASSERT_ALWAYS(connectionIndex < mSignalConnections.Count() && "DeleteConnection called with invalid index");
-
-  // delete the object
-  SignalConnection* connection(mSignalConnections[connectionIndex]);
-  delete connection;
 
   if(mEmittingFlag)
   {
-    // IMPORTANT - do not remove from items from mSignalConnections, set to NULL instead.
+    // IMPORTANT - do not remove from items from mSignalConnections, reset instead.
     // Signal Emit() methods require that connection count is not reduced while iterating
     // i.e. DeleteConnection can be called from within callbacks, while iterating through mSignalConnections.
-    mSignalConnections[connectionIndex] = nullptr;
+    mSignalConnections[connectionIndex] = {nullptr};
+    ++mNullConnections;
   }
   else
   {
     // If application connects and disconnects without the signal never emitting,
     // the mSignalConnections vector keeps growing and growing as CleanupConnections() is done from Emit.
-    mSignalConnections.Erase(mSignalConnections.Begin() + connectionIndex);
+    mSignalConnections.erase(mSignalConnections.begin() + connectionIndex);
   }
 }
 
 void BaseSignal::CleanupConnections()
 {
-  const std::size_t total = mSignalConnections.Count();
-  // only do something if there are items
-  if(total > 0)
+  if(!mSignalConnections.empty())
   {
-    std::size_t index = 0;
-    // process the whole vector
-    for(std::size_t i = 0; i < total; ++i)
-    {
-      if(mSignalConnections[index] == nullptr)
-      {
-        // items will be moved so don't increase index (erase will decrease the count of vector)
-        mSignalConnections.Erase(mSignalConnections.Begin() + index);
-      }
-      else
-      {
-        // increase to next element
-        ++index;
-      }
-    }
+    //Remove Signals that are already markeed nullptr.
+    mSignalConnections.erase(std::remove_if(mSignalConnections.begin(),
+                                            mSignalConnections.end(),
+                                            [](auto& elm) { return (elm) ? false : true; }),
+                             mSignalConnections.end());
   }
+  mNullConnections = 0;
 }
 
 // BaseSignal::EmitGuard
index ddfe915..359a5f3 100644 (file)
@@ -18,6 +18,9 @@
  *
  */
 
+// EXTERNAL INCLUDES
+#include <vector>
+
 // INTERNAL INCLUDES
 #include <dali/public-api/common/dali-common.h>
 #include <dali/public-api/common/dali-vector.h>
@@ -96,7 +99,10 @@ public:
    * @SINCE_1_0.0
    * @return True if there are any slots connected to the signal.
    */
-  bool Empty() const;
+  bool Empty() const
+  {
+    return (0 == GetConnectionCount());
+  }
 
   /**
    * @brief Queries the number of slots.
@@ -104,7 +110,10 @@ public:
    * @SINCE_1_0.0
    * @return The number of slots connected to this signal
    */
-  std::size_t GetConnectionCount() const;
+  std::size_t GetConnectionCount() const
+  {
+    return mSignalConnections.size() - mNullConnections;
+  }
 
   // Templated Emit functions for the Signal implementations
 
@@ -169,7 +178,7 @@ public:
 
     // If more connections are added by callbacks, these are ignore until the next Emit()
     // Note that count cannot be reduced while iterating
-    const std::size_t initialCount(mSignalConnections.Count());
+    const std::size_t initialCount(mSignalConnections.size());
 
     for(std::size_t i = 0; i < initialCount; ++i)
     {
@@ -208,7 +217,7 @@ public:
 
     // If more connections are added by callbacks, these are ignore until the next Emit()
     // Note that count cannot be reduced while iterating
-    const std::size_t initialCount(mSignalConnections.Count());
+    const std::size_t initialCount(mSignalConnections.size());
 
     for(std::size_t i = 0; i < initialCount; ++i)
     {
@@ -248,7 +257,7 @@ public:
 
     // If more connections are added by callbacks, these are ignore until the next Emit()
     // Note that count cannot be reduced while iterating
-    const std::size_t initialCount(mSignalConnections.Count());
+    const std::size_t initialCount(mSignalConnections.size());
 
     for(std::size_t i = 0; i < initialCount; ++i)
     {
@@ -288,7 +297,7 @@ public:
 
     // If more connections are added by callbacks, these are ignore until the next Emit()
     // Note that count cannot be reduced while iterating
-    const std::size_t initialCount(mSignalConnections.Count());
+    const std::size_t initialCount(mSignalConnections.size());
 
     for(std::size_t i = 0; i < initialCount; ++i)
     {
@@ -329,7 +338,7 @@ public:
 
     // If more connections are added by callbacks, these are ignore until the next Emit()
     // Note that count cannot be reduced while iterating
-    const std::size_t initialCount(mSignalConnections.Count());
+    const std::size_t initialCount(mSignalConnections.size());
 
     for(std::size_t i = 0; i < initialCount; ++i)
     {
@@ -370,7 +379,7 @@ public:
 
     // If more connections are added by callbacks, these are ignore until the next Emit()
     // Note that count cannot be reduced while iterating
-    const std::size_t initialCount(mSignalConnections.Count());
+    const std::size_t initialCount(mSignalConnections.size());
 
     for(std::size_t i = 0; i < initialCount; ++i)
     {
@@ -412,7 +421,7 @@ public:
 
     // If more connections are added by callbacks, these are ignore until the next Emit()
     // Note that count cannot be reduced while iterating
-    const std::size_t initialCount(mSignalConnections.Count());
+    const std::size_t initialCount(mSignalConnections.size());
 
     for(std::size_t i = 0; i < initialCount; ++i)
     {
@@ -482,7 +491,10 @@ private:
    * @param[in] connectionIndex The index of the callback
    * @return The callback, or NULL if the connection has been deleted
    */
-  CallbackBase* GetCallback(std::size_t connectionIndex) const;
+  CallbackBase* GetCallback(std::size_t connectionIndex) const noexcept
+  {
+    return mSignalConnections[connectionIndex].GetCallback();
+  }
 
   /**
    * @brief Helper to find whether a callback is connected.
@@ -491,7 +503,7 @@ private:
    * @param[in] callback The call back object
    * @return A valid index if the callback is connected
    */
-  int32_t FindCallback(CallbackBase* callback);
+  int32_t FindCallback(CallbackBase* callback) const noexcept;
 
   /**
    * @brief Deletes a connection object from the list of connections.
@@ -514,9 +526,9 @@ private:
   BaseSignal& operator=(BaseSignal&&) = delete;      ///< Deleted move assignment operator. @SINCE_1_9.25
 
 private:
-  Dali::Vector<SignalConnection*> mSignalConnections; ///< Array of connections
-
-  bool mEmittingFlag; ///< Used to guard against nested Emit() calls
+  std::vector<SignalConnection> mSignalConnections; ///< Array of connections
+  uint32_t                      mNullConnections{0}; ///< Empty Connections in the array.
+  bool                          mEmittingFlag{false}; ///< Used to guard against nested Emit() calls
 };
 
 /**