Merge my use-tsDLList branch, PR#176

anjohnson · Feb 22, 2022 · b2f0aec · b2f0aec
2 parents b1b6e77 + 5594d42
commit b2f0aec
Show file tree

Hide file tree

Showing 5 changed files with 45 additions and 42 deletions.
diff --git a/documentation/release_notes.dox b/documentation/release_notes.dox
@@ -1,5 +1,13 @@
 /** @page pvarelease_notes Release Notes
 
+Release 7.1.6 (UNRELEASED)
+=========================
+
+- Changes to caProvider
+  - More internal changes to improve performance when connecting tens of
+    thousands of CA channels.
+
+
 Release 7.1.5 (October 2021)
 ==========================
 

diff --git a/src/ca/caChannel.cpp b/src/ca/caChannel.cpp
@@ -116,6 +116,8 @@ CAChannel::CAChannel(std::string const & channelName,
     connectNotification(new Notification()),
     ca_context(channelProvider->caContext())
 {
+    if (channelName.empty())
+        throw std::invalid_argument("Channel name cannot be empty");
 }
 
 void CAChannel::activate(short priority)
@@ -128,25 +130,22 @@ void CAChannel::activate(short priority)
         ca_connection_handler, this,
         priority, // TODO mapping
         &channelID);
+    Status errorStatus;
     if (result == ECA_NORMAL) {
-        channelCreated = true;
+        epicsGuard<epicsMutex> G(requestsMutex);
+        channelCreated = true;      // Set before addChannel()
         CAChannelProviderPtr provider(channelProvider.lock());
         if (provider)
-            provider->addChannel(shared_from_this());
-        EXCEPTION_GUARD(req->channelCreated(Status::Ok, shared_from_this()));
+            provider->addChannel(*this);
     }
     else {
-        Status errorStatus(Status::STATUSTYPE_ERROR, string(ca_message(result)));
-        EXCEPTION_GUARD(req->channelCreated(errorStatus, shared_from_this()));
+        errorStatus = Status::error(ca_message(result));
     }
+    EXCEPTION_GUARD(req->channelCreated(errorStatus, shared_from_this()));
 }
 
 CAChannel::~CAChannel()
 {
-    {
-        epicsGuard<epicsMutex> G(requestsMutex);
-        if (!channelCreated) return;
-    }
     disconnectChannel();
 }
 
@@ -155,7 +154,10 @@ void CAChannel::disconnectChannel()
     {
         epicsGuard<epicsMutex> G(requestsMutex);
         if (!channelCreated) return;
-        channelCreated = false;
+        CAChannelProviderPtr provider(channelProvider.lock());
+        if (provider)
+            provider->delChannel(*this);
+        channelCreated = false;     // Clear only after delChannel()
     }
     std::vector<CAChannelMonitorWPtr>::iterator it;
     for (it = monitorlist.begin(); it!=monitorlist.end(); ++it) {
@@ -167,10 +169,11 @@ void CAChannel::disconnectChannel()
     /* Clear CA Channel */
     Attach to(ca_context);
     int result = ca_clear_channel(channelID);
-    if (result == ECA_NORMAL) return;
-    string mess("CAChannel::disconnectChannel() ");
-    mess += ca_message(result);
-    cerr << mess << endl;
+    if (result != ECA_NORMAL) {
+        string mess("CAChannel::disconnectChannel() ");
+        mess += ca_message(result);
+        cerr << mess << endl;
+    }
 }
 
 chid CAChannel::getChannelID()

diff --git a/src/ca/caChannel.h b/src/ca/caChannel.h
@@ -18,6 +18,7 @@
 #include <epicsMutex.h>
 #include <epicsEvent.h>
 #include <cadef.h>
+#include <tsDLList.h>
 
 #include <pv/pvAccess.h>
 
@@ -66,6 +67,7 @@ class CAChannelGetField :
 
 class CAChannel :
     public Channel,
+    public tsDLNode<CAChannel>,
     public NotifierClient,
     public std::tr1::enable_shared_from_this<CAChannel>
 {

diff --git a/src/ca/caProvider.cpp b/src/ca/caProvider.cpp
@@ -31,22 +31,12 @@ CAChannelProvider::CAChannelProvider(const std::tr1::shared_ptr<Configuration> &
 
 CAChannelProvider::~CAChannelProvider()
 {
-    std::queue<CAChannelPtr> channelQ;
-    {
-        std::vector<CAChannelWPtr>::iterator it;
-        epicsGuard<epicsMutex> G(channelListMutex);
-        for (it = caChannelList.begin(); it != caChannelList.end(); ++it)
-        {
-            CAChannelPtr caChannel(it->lock());
-            if (caChannel)
-                channelQ.push(caChannel);
-        }
-        caChannelList.clear();
-    }
-    while (!channelQ.empty())
-    {
-        channelQ.front()->disconnectChannel();
-        channelQ.pop();
+    epicsGuard<epicsMutex> G(channelListMutex);
+    while (CAChannel *ch = caChannelList.get()) {
+        // Here disconnectChannel() can't call our delChannel()
+        // beacuse its CAChannelProviderPtr has by now expired.
+        // That's why we removed it from caChannelList above.
+        ch->disconnectChannel();
     }
 }
 
@@ -106,18 +96,16 @@ Channel::shared_pointer CAChannelProvider::createChannel(
     return CAChannel::create(shared_from_this(), channelName, priority, channelRequester);
 }
 
-void CAChannelProvider::addChannel(const CAChannelPtr &channel)
+void CAChannelProvider::addChannel(CAChannel &channel)
 {
-    std::vector<CAChannelWPtr>::iterator it;
     epicsGuard<epicsMutex> G(channelListMutex);
-    for (it = caChannelList.begin(); it != caChannelList.end(); ++it)
-    {
-        if (it->expired()) {
-            *it = channel;
-            return;
-        }
-    }
-    caChannelList.push_back(channel);
+    caChannelList.add(channel);
+}
+
+void CAChannelProvider::delChannel(CAChannel &channel)
+{
+    epicsGuard<epicsMutex> G(channelListMutex);
+    caChannelList.remove(channel);
 }
 
 void CAChannelProvider::configure(epics::pvData::PVStructure::shared_pointer /*configuration*/)

diff --git a/src/ca/caProviderPvt.h b/src/ca/caProviderPvt.h
@@ -14,6 +14,7 @@
 
 #include <cadef.h>
 #include <epicsMutex.h>
+#include <tsDLList.h>
 
 #include <pv/logger.h>
 #include <pv/pvAccess.h>
@@ -80,7 +81,8 @@ class CAChannelProvider :
     virtual void flush();
     virtual void poll();
 
-    void addChannel(const CAChannelPtr & channel);
+    void addChannel(CAChannel &channel);
+    void delChannel(CAChannel &channel);
 
     CAContextPtr caContext() {
         return ca_context;
@@ -94,7 +96,7 @@ class CAChannelProvider :
 private:
     CAContextPtr ca_context;
     epicsMutex channelListMutex;
-    std::vector<CAChannelWPtr> caChannelList;
+    tsDLList<CAChannel> caChannelList;
 
     NotifierConveyor connectNotifier;
     NotifierConveyor resultNotifier;