base-signal connect and disconnect in O(1) 17/274617/13
authorEunki, Hong <eunkiki.hong@samsung.com>
Tue, 3 May 2022 14:24:22 +0000 (23:24 +0900)
committerEunki, Hong <eunkiki.hong@samsung.com>
Mon, 16 May 2022 10:39:01 +0000 (19:39 +0900)
Make base-signal control the connection in O(1)
by std::unordered_map and std::list<>::iterator.
and make connection-tracker control the connection in O(1)
by std::unordered_map

std::unordered_map can found duplicated value fast
and std::list<>::iterator can access & remove value fast.
And also, std::list can keep ordered by inputed time.

TODO : Remove the dependency of std::vector header in dali-signal.h
is quite big process. Will be done in other patch

Change-Id: I940d33c46b1470973219c7a9fb10ac89dd70f1ff
Signed-off-by: Eunki, Hong <eunkiki.hong@samsung.com>
dali/public-api/signals/base-signal.cpp
dali/public-api/signals/base-signal.h
dali/public-api/signals/connection-tracker.cpp
dali/public-api/signals/connection-tracker.h
dali/public-api/signals/dali-signal.h

index a8c55ad..5e7b66c 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2021 Samsung Electronics Co., Ltd.
+ * Copyright (c) 2022 Samsung Electronics Co., Ltd.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
 #include <dali/public-api/signals/base-signal.h>
 
 // EXTERNAL INCLUDES
-#include <algorithm> // remove_if
+#include <unordered_map>
 
 // INTERNAL INCLUDES
 #include <dali/integration-api/debug.h>
 
 namespace
 {
-const int32_t INVALID_CALLBACK_INDEX = -1;
-
+struct CallbackBasePtrHash
+{
+  std::size_t operator()(const Dali::CallbackBase* callback) const noexcept
+  {
+    std::size_t functionHash = reinterpret_cast<std::size_t>(reinterpret_cast<void*>(callback->mFunction));
+    std::size_t objectHash   = reinterpret_cast<std::size_t>(reinterpret_cast<void*>(callback->mImpl.mObjectPointer));
+    return functionHash ^ objectHash;
+  }
+};
+struct CallbackBasePtrEqual
+{
+  bool operator()(const Dali::CallbackBase* lhs, const Dali::CallbackBase* rhs) const noexcept
+  {
+    return (*lhs) == (*rhs);
+  }
+};
 } // unnamed namespace
 
 namespace Dali
 {
+/**
+ * @brief Extra struct for callback base cache.
+ */
+struct BaseSignal::Impl
+{
+  Impl()  = default;
+  ~Impl() = default;
+
+  /**
+   * @brief Get the iterator of connections list by the callback base pointer.
+   * Note that we should compare the 'value' of callback, not pointer.
+   * So, we need to define custom hash & compare functor of callback base pointer.
+   */
+  std::unordered_map<const CallbackBase*, std::list<SignalConnection>::iterator, CallbackBasePtrHash, CallbackBasePtrEqual> mCallbackCache;
+};
+
 BaseSignal::BaseSignal()
-: mEmittingFlag(false)
+: mCacheImpl(new BaseSignal::Impl()),
+  mEmittingFlag(false)
 {
 }
 
@@ -53,10 +84,9 @@ 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.size());
-  for(std::size_t i = 0; i < count; i++)
+  for(auto iter = mSignalConnections.begin(), iterEnd = mSignalConnections.end(); iter != iterEnd; ++iter)
   {
-    auto& connection = mSignalConnections[i];
+    auto& connection = *iter;
 
     // Note that values are set to NULL in DeleteConnection
     if(connection)
@@ -64,21 +94,23 @@ BaseSignal::~BaseSignal()
       connection.Disconnect(this);
     }
   }
+
+  delete mCacheImpl;
 }
 
 void BaseSignal::OnConnect(CallbackBase* callback)
 {
   DALI_ASSERT_ALWAYS(nullptr != callback && "Invalid member function pointer passed to Connect()");
 
-  int32_t index = FindCallback(callback);
+  auto iter = FindCallback(callback);
 
   // Don't double-connect the same callback
-  if(INVALID_CALLBACK_INDEX == index)
+  if(iter == mSignalConnections.end())
   {
-    // create a new signal connection object, to allow the signal to track the connection.
-    //SignalConnection* connection = new SignalConnection(callback);
+    auto newIter = mSignalConnections.insert(mSignalConnections.end(), SignalConnection(callback));
 
-    mSignalConnections.push_back(SignalConnection(callback));
+    // Store inserted iterator for this callback
+    mCacheImpl->mCallbackCache[callback] = newIter;
   }
   else
   {
@@ -91,11 +123,11 @@ void BaseSignal::OnDisconnect(CallbackBase* callback)
 {
   DALI_ASSERT_ALWAYS(nullptr != callback && "Invalid member function pointer passed to Disconnect()");
 
-  int32_t index = FindCallback(callback);
+  auto iter = FindCallback(callback);
 
-  if(index > INVALID_CALLBACK_INDEX)
+  if(iter != mSignalConnections.end())
   {
-    DeleteConnection(index);
+    DeleteConnection(iter);
   }
 
   // call back is a temporary created to find which slot should be disconnected.
@@ -107,15 +139,15 @@ void BaseSignal::OnConnect(ConnectionTrackerInterface* tracker, CallbackBase* ca
   DALI_ASSERT_ALWAYS(nullptr != tracker && "Invalid ConnectionTrackerInterface pointer passed to Connect()");
   DALI_ASSERT_ALWAYS(nullptr != callback && "Invalid member function pointer passed to Connect()");
 
-  int32_t index = FindCallback(callback);
+  auto iter = FindCallback(callback);
 
   // Don't double-connect the same callback
-  if(INVALID_CALLBACK_INDEX == index)
+  if(iter == mSignalConnections.end())
   {
-    // create a new signal connection object, to allow the signal to track the connection.
-    //SignalConnection* connection = new SignalConnection(tracker, callback);
+    auto newIter = mSignalConnections.insert(mSignalConnections.end(), {tracker, callback});
 
-    mSignalConnections.push_back({tracker, callback});
+    // Store inserted iterator for this callback
+    mCacheImpl->mCallbackCache[callback] = newIter;
 
     // Let the connection tracker know that a connection between a signal and a slot has been made.
     tracker->SignalConnected(this, callback);
@@ -132,15 +164,16 @@ void BaseSignal::OnDisconnect(ConnectionTrackerInterface* tracker, CallbackBase*
   DALI_ASSERT_ALWAYS(nullptr != tracker && "Invalid ConnectionTrackerInterface pointer passed to Disconnect()");
   DALI_ASSERT_ALWAYS(nullptr != callback && "Invalid member function pointer passed to Disconnect()");
 
-  int32_t index = FindCallback(callback);
+  auto iter = FindCallback(callback);
 
-  if(index > INVALID_CALLBACK_INDEX)
+  if(iter != mSignalConnections.end())
   {
     // temporary pointer to disconnected callback
-    CallbackBase* disconnectedCallback = mSignalConnections[index].GetCallback();
+    // Note that (*iter).GetCallback() != callback is possible.
+    CallbackBase* disconnectedCallback = (*iter).GetCallback();
 
     // close the signal side connection first.
-    DeleteConnection(index);
+    DeleteConnection(iter);
 
     // close the slot side connection
     tracker->SignalDisconnected(this, disconnectedCallback);
@@ -153,62 +186,55 @@ void BaseSignal::OnDisconnect(ConnectionTrackerInterface* tracker, CallbackBase*
 // for SlotObserver::SlotDisconnected
 void BaseSignal::SlotDisconnected(CallbackBase* callback)
 {
-  const std::size_t count(mSignalConnections.size());
-  for(std::size_t i = 0; i < count; ++i)
-  {
-    const CallbackBase* connectionCallback = GetCallback(i);
-
-    // Pointer comparison i.e. SignalConnection contains pointer to same callback instance
-    if(connectionCallback &&
-       connectionCallback == callback)
-    {
-      DeleteConnection(i);
+  DALI_ASSERT_ALWAYS(nullptr != callback && "Invalid callback function passed to SlotObserver::SlotDisconnected()");
 
-      // Disconnection complete
-      return;
-    }
+  auto iter = FindCallback(callback);
+  if(DALI_LIKELY(iter != mSignalConnections.end()))
+  {
+    DeleteConnection(iter);
+    return;
   }
 
   DALI_ABORT("Callback lost in SlotDisconnected()");
 }
 
-int32_t BaseSignal::FindCallback(CallbackBase* callback) const noexcept
+std::list<SignalConnection>::iterator BaseSignal::FindCallback(CallbackBase* callback) noexcept
 {
-  int32_t index(INVALID_CALLBACK_INDEX);
+  const auto& convertorIter = mCacheImpl->mCallbackCache.find(callback);
 
-  // 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)
-  for(auto i = 0u; i < mSignalConnections.size(); ++i)
+  if(convertorIter != mCacheImpl->mCallbackCache.end())
   {
-    const CallbackBase* connectionCallback = GetCallback(i);
+    const auto& iter = convertorIter->second; // std::list<SignalConnection>::iterator
 
-    // Note that values are set to NULL in DeleteConnection
-    if(connectionCallback && (*connectionCallback == *callback))
+    if(*iter) // the value of iterator can be null.
     {
-      index = static_cast<int>(i); // only 2,147,483,647 connections supported, no error check
-      break;
+      if(*(iter->GetCallback()) == *callback)
+      {
+        return iter;
+      }
     }
   }
-
-  return index;
+  return mSignalConnections.end();
 }
 
-void BaseSignal::DeleteConnection(std::size_t connectionIndex)
+void BaseSignal::DeleteConnection(std::list<SignalConnection>::iterator iter)
 {
+  // Erase cache first.
+  mCacheImpl->mCallbackCache.erase(iter->GetCallback());
+
   if(mEmittingFlag)
   {
     // 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};
+    (*iter) = {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(iter);
   }
 }
 
@@ -217,10 +243,7 @@ void BaseSignal::CleanupConnections()
   if(!mSignalConnections.empty())
   {
     //Remove Signals that are already markeed nullptr.
-    mSignalConnections.erase(std::remove_if(mSignalConnections.begin(),
-                                            mSignalConnections.end(),
-                                            [](auto& elm) { return (elm) ? false : true; }),
-                             mSignalConnections.end());
+    mSignalConnections.remove_if([](auto& elem) { return (elem) ? false : true; });
   }
   mNullConnections = 0;
 }
index b63e1e2..5420952 100644 (file)
@@ -2,7 +2,7 @@
 #define DALI_BASE_SIGNAL_H
 
 /*
- * Copyright (c) 2021 Samsung Electronics Co., Ltd.
+ * Copyright (c) 2022 Samsung Electronics Co., Ltd.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
  */
 
 // EXTERNAL INCLUDES
-#include <vector>
+#include <stdint.h> // for uint32_t
 
 // INTERNAL INCLUDES
 #include <dali/public-api/common/dali-common.h>
-#include <dali/public-api/common/dali-vector.h>
+#include <dali/public-api/common/list-wrapper.h>
 #include <dali/public-api/signals/callback.h>
 #include <dali/public-api/signals/connection-tracker-interface.h>
 #include <dali/public-api/signals/signal-slot-connections.h>
@@ -175,14 +175,12 @@ 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.size());
-
-    for(std::size_t i = 0; i < initialCount; ++i)
+    auto count = mSignalConnections.size();
+    auto iter  = mSignalConnections.begin();
+    while(count--)
     {
-      CallbackBase* callback(GetCallback(i));
-
-      // Note that connections will be set to NULL when disconnected
-      // This is preferable to reducing the connection count while iterating
+      CallbackBase* callback((*iter) ? iter->GetCallback() : nullptr);
+      ++iter;
       if(callback)
       {
         returnVal = CallbackBase::ExecuteReturn<Ret, Args...>(*callback, args...);
@@ -222,14 +220,12 @@ 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.size());
-
-    for(std::size_t i = 0; i < initialCount; ++i)
+    auto count = mSignalConnections.size();
+    auto iter  = mSignalConnections.begin();
+    while(count--)
     {
-      CallbackBase* callback(GetCallback(i));
-
-      // Note that connections will be set to NULL when disconnected
-      // This is preferable to reducing the connection count while iterating
+      CallbackBase* callback((*iter) ? iter->GetCallback() : nullptr);
+      ++iter;
       if(callback)
       {
         CallbackBase::Execute<Args...>(*callback, args...);
@@ -288,33 +284,21 @@ private: // SlotObserver interface, to be told when a slot disconnects
 
 private:
   /**
-   * @brief Returns a callback given an index in to the connection array.
-   *
-   * @SINCE_1_0.0
-   * @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 noexcept
-  {
-    return mSignalConnections[connectionIndex].GetCallback();
-  }
-
-  /**
    * @brief Helper to find whether a callback is connected.
    *
-   * @SINCE_1_0.0
+   * @SINCE_2_1.22
    * @param[in] callback The call back object
    * @return A valid index if the callback is connected
    */
-  int32_t FindCallback(CallbackBase* callback) const noexcept;
+  std::list<SignalConnection>::iterator FindCallback(CallbackBase* callback) noexcept;
 
   /**
    * @brief Deletes a connection object from the list of connections.
    *
-   * @SINCE_1_0.0
-   * @param[in] connectionIndex The index of the callback
+   * @SINCE_2_1.22
+   * @param[in] iter The index of the callback
    */
-  void DeleteConnection(std::size_t connectionIndex);
+  void DeleteConnection(std::list<SignalConnection>::iterator iter);
 
   /**
    * @brief Helper to remove NULL items from mSignalConnections, which is only safe at the end of Emit()
@@ -329,10 +313,14 @@ private:
   BaseSignal& operator=(BaseSignal&&) = delete;      ///< Deleted move assignment operator. @SINCE_1_9.25
 
 private:
-  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.
-  bool*                         mSignalDeleted{nullptr}; ///< Used to guard against deletion during Emit() calls.
+  struct DALI_INTERNAL Impl;
+  Impl*                mCacheImpl; ///< Private internal extra data.
+
+private:
+  std::list<SignalConnection> mSignalConnections{};    ///< List of connections
+  uint32_t                    mNullConnections{0};     ///< Empty Connections in the array.
+  bool                        mEmittingFlag{false};    ///< Used to guard against nested Emit() calls.
+  bool*                       mSignalDeleted{nullptr}; ///< Used to guard against deletion during Emit() calls.
 };
 
 /**
index 5ff2732..42ed2d5 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2021 Samsung Electronics Co., Ltd.
+ * Copyright (c) 2022 Samsung Electronics Co., Ltd.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
 // CLASS HEADER
 #include <dali/public-api/signals/connection-tracker.h>
 
+// EXTERNAL INCLUDES
+#include <unordered_map>
+
+// INTERNAL INCLUDES
 #include <dali/public-api/signals/callback.h>
-#include <dali/public-api/signals/signal-slot-connections.h>
 #include <dali/public-api/signals/signal-slot-observers.h>
 
 namespace Dali
 {
-ConnectionTracker::ConnectionTracker() = default;
+/**
+ * @brief Extra struct for callback base cache.
+ */
+struct ConnectionTracker::Impl
+{
+  Impl()  = default;
+  ~Impl() = default;
+
+  std::unordered_map<CallbackBase*, SlotObserver*> mCallbackCache;
+};
+
+ConnectionTracker::ConnectionTracker()
+: mCacheImpl(new ConnectionTracker::Impl())
+{
+}
 
 ConnectionTracker::~ConnectionTracker()
 {
   DisconnectAll();
+  delete mCacheImpl;
 }
 
 void ConnectionTracker::DisconnectAll()
 {
-  std::size_t size = mConnections.Size();
-
-  for(std::size_t i = 0; i < size; ++i)
+  // Iterate unordered list of CallbackBase / SlotObserver.
+  // Note that we don't need to keep order of ConnectionTracker::SignalConnected
+  for(auto iter = mCacheImpl->mCallbackCache.begin(), iterEnd = mCacheImpl->mCallbackCache.end(); iter != iterEnd; ++iter)
   {
-    auto& connection = mConnections[i];
+    auto& callbackBase = iter->first;
+    auto& slotObserver = iter->second;
 
     // Tell the signal that the slot is disconnected
-    connection.GetSlotObserver()->SlotDisconnected(connection.GetCallback());
+    slotObserver->SlotDisconnected(callbackBase);
   }
 
-  mConnections.Clear();
+  mCacheImpl->mCallbackCache.clear();
+  mCacheImpl->mCallbackCache.rehash(0); ///< Note : unordered_map.clear() didn't deallocate memory.
 }
 
 void ConnectionTracker::SignalConnected(SlotObserver* slotObserver, CallbackBase* callback)
 {
-  mConnections.PushBack(SlotConnection(slotObserver, callback));
+  // We can assume that there is no duplicated callback come here
+  mCacheImpl->mCallbackCache[callback] = slotObserver;
 }
 
-void ConnectionTracker::SignalDisconnected(SlotObserver* signal, CallbackBase* callback)
+void ConnectionTracker::SignalDisconnected(SlotObserver* slotObserver, CallbackBase* callback)
 {
-  std::size_t size = mConnections.Size();
-
-  for(std::size_t i = 0; i < size; ++i)
+  // Remove from CallbackBase / SlotObserver list
+  const bool isRemoved = mCacheImpl->mCallbackCache.erase(callback);
+  if(DALI_LIKELY(isRemoved))
   {
-    auto& connection = mConnections[i];
-
-    // Pointer comparison i.e. SignalConnection contains pointer to same callback instance
-    if(connection.GetCallback() == callback)
-    {
-      // Remove from connection list
-      mConnections.Erase(mConnections.Begin() + i);
-
-      // Disconnection complete
-      return;
-    }
+    // Disconnection complete
+    return;
   }
 
   DALI_ABORT("Callback lost in SignalDisconnected()");
@@ -75,7 +87,7 @@ void ConnectionTracker::SignalDisconnected(SlotObserver* signal, CallbackBase* c
 
 std::size_t ConnectionTracker::GetConnectionCount() const
 {
-  return mConnections.Size();
+  return mCacheImpl->mCallbackCache.size();
 }
 
 } // namespace Dali
index d2f6262..6232d66 100644 (file)
@@ -2,7 +2,7 @@
 #define DALI_CONNECTION_TRACKER_H
 
 /*
- * Copyright (c) 2020 Samsung Electronics Co., Ltd.
+ * Copyright (c) 2022 Samsung Electronics Co., Ltd.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
  *
  */
 
+// EXTERNAL INCLUDES
+#include <cstddef> // for std::size_t
+
 // INTERNAL INCLUDES
 #include <dali/public-api/common/dali-common.h>
-#include <dali/public-api/common/dali-vector.h>
 #include <dali/public-api/signals/connection-tracker-interface.h>
-#include <dali/public-api/signals/signal-slot-connections.h>
+#include <dali/public-api/signals/signal-slot-observers.h>
 
 namespace Dali
 {
@@ -33,7 +35,6 @@ namespace Dali
 
 class CallbackBase;
 class SlotObserver;
-class SlotConnection;
 
 /**
  * @brief Connection tracker concrete implementation.
@@ -84,7 +85,8 @@ private:
   ConnectionTracker& operator=(ConnectionTracker&&) = delete;      ///< Deleted move assignment operator. @SINCE_1_9.25
 
 private:
-  Dali::Vector<SlotConnection> mConnections; ///< Vector of connection
+  struct DALI_INTERNAL Impl;
+  Impl*                mCacheImpl; ///< Private internal extra data.
 };
 
 /**
index c4fd2ed..2c893c7 100644 (file)
@@ -2,7 +2,7 @@
 #define DALI_SIGNAL_H
 
 /*
- * Copyright (c) 2020 Samsung Electronics Co., Ltd.
+ * Copyright (c) 2022 Samsung Electronics Co., Ltd.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -57,6 +57,7 @@
 
 // INTERNAL INCLUDES
 #include <dali/public-api/common/dali-common.h>
+#include <dali/public-api/common/vector-wrapper.h>
 #include <dali/public-api/signals/base-signal.h>
 #include <dali/public-api/signals/callback.h>
 #include <dali/public-api/signals/signal-slot-connections.h>