pcbnew: refactor connectivity locking

Locks protect the std::set in each item.  Devolving the mutex to the
CN_ITEM allows multiple threads to make simultaneous connections to
different items where they do not conflict.

pcbnew/connectivity/connectivity_algo.cpp

@@ -233,7 +233,7 @@ void CN_CONNECTIVITY_ALGO::searchConnections()
                             i < dirtyItems.size();
                             i = nextItem.fetch_add( 1 ) )
                 {
-                    CN_VISITOR visitor( dirtyItems[i], &m_listLock );
+                    CN_VISITOR visitor( dirtyItems[i] );
                     m_itemList.FindNearby( dirtyItems[i], visitor );
 
                     if( m_progressReporter )
@@ -597,8 +597,8 @@ void CN_VISITOR::checkZoneItemConnection( CN_ZONE* aZone, CN_ITEM* aItem )
             ( aItem->Parent()->Type() == PCB_TRACE_T &&
               zoneItem->ContainsPoint( aItem->GetAnchor( 1 ) ) ) )
     {
-        std::lock_guard<std::mutex> lock( *m_listLock );
+        zoneItem->Connect( aItem );
-        CN_ITEM::Connect( zoneItem, aItem );
+        aItem->Connect( zoneItem );
     }
 }
 
@@ -622,8 +622,8 @@ void CN_VISITOR::checkZoneZoneConnection( CN_ZONE* aZoneA, CN_ZONE* aZoneB )
     {
         if( aZoneB->ContainsPoint( outline.CPoint( i ) ) )
         {
-            std::lock_guard<std::mutex> lock( *m_listLock );
+            aZoneA->Connect( aZoneB );
-            CN_ITEM::Connect( aZoneA, aZoneB );
+            aZoneB->Connect( aZoneA );
             return;
         }
     }
@@ -634,8 +634,8 @@ void CN_VISITOR::checkZoneZoneConnection( CN_ZONE* aZoneA, CN_ZONE* aZoneB )
     {
         if( aZoneA->ContainsPoint( outline2.CPoint( i ) ) )
         {
-            std::lock_guard<std::mutex> lock( *m_listLock );
+            aZoneA->Connect( aZoneB );
-            CN_ITEM::Connect( aZoneA, aZoneB );
+            aZoneB->Connect( aZoneA );
             return;
         }
     }
@@ -693,8 +693,8 @@ bool CN_VISITOR::operator()( CN_ITEM* aCandidate )
             ( parentA->Type() == PCB_TRACE_T && parentB->HitTest( ptA2 ) ) ||
             ( parentB->Type() == PCB_TRACE_T && parentA->HitTest( ptB2 ) ) )
     {
-        std::lock_guard<std::mutex> lock( *m_listLock );
+        m_item->Connect( aCandidate );
-        CN_ITEM::Connect( m_item, aCandidate );
+        aCandidate->Connect( m_item );
     }
 
     return true;

pcbnew/connectivity/connectivity_algo.h

@@ -115,7 +115,7 @@ private:
 
     class ITEM_MAP_ENTRY
     {
-public:
+    public:
         ITEM_MAP_ENTRY( CN_ITEM* aItem = nullptr )
         {
             if( aItem )
@@ -143,11 +143,8 @@ public:
         std::list<CN_ITEM*> m_items;
     };
 
-    std::mutex m_listLock;
     CN_LIST m_itemList;
 
-    using ITEM_MAP_PAIR = std::pair <const BOARD_CONNECTED_ITEM*, ITEM_MAP_ENTRY>;
-
     std::unordered_map<const BOARD_CONNECTED_ITEM*, ITEM_MAP_ENTRY> m_itemMap;
 
     CLUSTERS m_connClusters;
@@ -252,9 +249,6 @@ public:
 
 };
 
-bool operator<( const CN_ANCHOR_PTR& a, const CN_ANCHOR_PTR& b );
-
-
 /**
  * Struct CN_VISTOR
  **/
@@ -262,9 +256,8 @@ class CN_VISITOR {
 
 public:
 
-    CN_VISITOR( CN_ITEM* aItem, std::mutex* aListLock ) :
+    CN_VISITOR( CN_ITEM* aItem ) :
-        m_item( aItem ),
+        m_item( aItem )
-        m_listLock( aListLock )
     {}
 
     bool operator()( CN_ITEM* aCandidate );
@@ -277,10 +270,6 @@ protected:
 
     ///> the item we are looking for connections to
     CN_ITEM* m_item;
-
-    ///> the mutex protecting our connection list
-    std::mutex* m_listLock;
-
 };
 
 #endif

pcbnew/connectivity/connectivity_items.h

@@ -171,6 +171,9 @@ private:
     ///> valid flag, used to identify garbage items (we use lazy removal)
     bool m_valid;
 
+    ///> mutex protecting this item's connected_items set to allow parallel connection threads
+    std::mutex m_listLock;
+
 protected:
     ///> dirty flag, used to identify recently added item not yet scanned into the connectivity search
     bool m_dirty;
@@ -267,17 +270,6 @@ public:
         return Layers().Start();
     }
 
-    /**
-     * Function LayersOverlap()
-     *
-     * Returns true if the set of layers spanned by aOther overlaps our
-     * layers.
-     */
-    bool LayersOverlap( const CN_ITEM* aOther ) const
-    {
-        return Layers().Overlaps( aOther->Layers() );
-    }
-
     const BOX2I& BBox()
     {
         if( m_dirty && m_valid )
@@ -318,15 +310,10 @@ public:
         return m_canChangeNet;
     }
 
-    bool isConnected( CN_ITEM* aItem ) const
+    void Connect( CN_ITEM* b )
     {
-        return ( m_connected.find( aItem ) != m_connected.end() );
+        std::lock_guard<std::mutex> lock( m_listLock );
-    }
+        m_connected.insert( b );
-
-    static void Connect( CN_ITEM* a, CN_ITEM* b )
-    {
-        a->m_connected.insert( b );
-        b->m_connected.insert( a );
     }
 
     void RemoveInvalidRefs();