Allegro: multithread zone fill handler for 70% loading speedup (VCU118)

Intersecting and fracturing zone fills takes forever for complex
fills (VCU118 has some fills severa with hundreds of thousands of
points).

However, they are trivially parallelisable - so do that and cut VCU
board load times by 70% - YMMV depending on CPU.

Further save 10% by sorting the heaviest zones first which is nice,
but the real thing to avoid is accidentally scheduling the biggest
zones consecutively on the same thread, which could be a substantial
penalty.

This has the happy effect of reducing the Allegro test suite to under
a minute (VCU118 dominates)

include/priority_thread_pool_task.h

@@ -0,0 +1,131 @@
+/*
+ * This program source code file is part of KiCad, a free EDA CAD application.
+ *
+ * Copyright The KiCad Developers, see AUTHORS.txt for contributors.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, you may find one here:
+ * http://www.gnu.org/licenses/old-licenses/gpl-2.0.html
+ * or you may search the http://www.gnu.org website for the version 2 license,
+ * or you may write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA
+ */
+
+#pragma once
+
+#include <thread_pool.h>
+
+
+/**
+ * A helper class to execute tasks on a thread pool in priority order, with progress reporting.
+ */
+template <typename ContainerT>
+class PRIORITY_THREAD_POOL_TASK
+{
+public:
+    using ItemT = typename ContainerT::value_type;
+
+    PRIORITY_THREAD_POOL_TASK() :
+            m_reporter( nullptr ),
+            m_highestPriority( BS::pr::high ),
+            m_reporterInterval( std::chrono::milliseconds( 250 ) )
+    {
+    }
+
+    void SetReporter( PROGRESS_REPORTER* aReporter ) { m_reporter = aReporter; }
+
+    /**
+     * Call this to execute the task on all items in aItems, using the thread pool
+     * and dispatching the tasks in order of descending priority as determined by
+     * comparePriority() implemented by the derived class.
+     */
+    void Execute( ContainerT& aItems )
+    {
+        thread_pool&                     tp = GetKiCadThreadPool();
+        std::vector<std::future<size_t>> returns;
+
+        // Compute priority keys paired to item indices
+        using IndexedPriority = std::pair<size_t, int>;
+        std::vector<IndexedPriority> indexedKeys( aItems.size() );
+        for( size_t i = 0; i < aItems.size(); ++i )
+        {
+            indexedKeys[i] = { i, computePriorityKey( aItems[i] ) };
+        }
+
+        // Sort by descending priority key
+        std::sort( indexedKeys.begin(), indexedKeys.end(),
+                   []( const IndexedPriority& a, const IndexedPriority& b )
+                   {
+                       return a.second > b.second;
+                   } );
+
+        // Dispatch largest first
+        const size_t numItems = aItems.size();
+        for( size_t priorityRank = 0; priorityRank < numItems; ++priorityRank )
+        {
+            const size_t itemIndex = indexedKeys[priorityRank].first;
+            ItemT&       item = aItems[itemIndex];
+
+            // Earlier ranking -> higher key -> should be higher priority
+            const size_t priority = ( ( numItems - priorityRank - 1 ) * m_highestPriority ) / numItems;
+
+            returns.emplace_back( tp.submit_task(
+                    [this, &item]
+                    {
+                        return task( item );
+                    },
+                    priority ) );
+        }
+
+        for( const std::future<size_t>& ret : returns )
+        {
+            std::future_status status = ret.wait_for( m_reporterInterval );
+
+            while( status != std::future_status::ready )
+            {
+                if( m_reporter )
+                    m_reporter->KeepRefreshing();
+
+                status = ret.wait_for( m_reporterInterval );
+            }
+        }
+    }
+
+protected:
+    PROGRESS_REPORTER* m_reporter;
+
+private:
+    /**
+     * Implement this to compute a priority key for an item.
+     *
+     * Return a number representing priority, where a higher number means higher priority.
+     * The actual values returned don't matter, only their relative order.
+     *
+     * (A relational a < b comparator would work too, but a unary key lets us compute it
+     * once per item in O(n) and then sort indices cheaply, rather than calling it O(n log n)
+     * times inside std::sort.)
+     *
+     * If you'd like to test the effect of this priority ordering, you
+     * can return a constant value to disable sorting, or return the inverse
+     * to sort backwards.
+     */
+    virtual int computePriorityKey( const ItemT& aItem ) const = 0;
+
+    /**
+     * Process one item in the thread pool.  Return the number of items processed.
+     */
+    virtual size_t task( ItemT& item ) = 0;
+
+    BS::priority_t            m_highestPriority;
+    std::chrono::milliseconds m_reporterInterval;
+};

pcbnew/pcb_io/allegro/allegro_builder.cpp

@@ -49,6 +49,7 @@
 #include <pcb_text.h>
 #include <pcb_shape.h>
 #include <pcb_track.h>
+#include <priority_thread_pool_task.h>
 #include <zone.h>
 #include <zone_utils.h>
 
@@ -3834,8 +3835,99 @@ static LSET getRuleAreaLayers( const LAYER_INFO& aLayerInfo, PCB_LAYER_ID aDefau
 }
 
 
+/**
+ * Filled zones have their own outline and the fill itself comes from
+ * a bunch of "related" spaces. To convert this to a KiCad-ish ZONE,
+ * we need to chop out only the bit of the wider filled zone that applies
+ * to the outline (i.e. intersection).
+ *
+ * Then that fill has to be fractured.
+ *
+ * This is all repeated for each layer's separated filled areas.
+ *
+ * This takes ages, so this class handles the information you need to collect to do that
+ * later on in a thread pool, and does that.
+ */
+class BOARD_BUILDER::ZONE_FILL_HANDLER
+{
+public:
+    /**
+     * This is all the info needed to do the fill of one layer of one zone.
+     */
+    struct FILL_INFO
+    {
+        ZONE*        m_Zone;
+        PCB_LAYER_ID m_Layer;
+        /// The wider filled area we will chop a piece out of for this layer of this zone
+        SHAPE_POLY_SET m_CombinedFill;
+    };
+
+    /**
+     * Priority task dispatcher for zone fills - we want to do the biggest ones first.
+     *
+     * On average, sorting the largest zones first is slightly faster (5-10%) on large boards.
+     *
+     * However, if you allow the large zones to be assigned at random, as they basically will be
+     * if they are handled later in the process, there's a chance the very biggest zones will end up
+     * consecutive in the same thread which could be a substantial penalty.
+     */
+    class COMPLEX_FIRST_FILL_TASK: public PRIORITY_THREAD_POOL_TASK<std::vector<FILL_INFO>>
+    {
+    public:
+        COMPLEX_FIRST_FILL_TASK( bool aSimplify ) : m_simplify( aSimplify ) {}
+
+    private:
+        int computePriorityKey( const FILL_INFO& a ) const override
+        {
+            return static_cast<int>( a.m_CombinedFill.TotalVertices() );
+        }
+
+        size_t task( FILL_INFO& fillInfo ) override
+        {
+            SHAPE_POLY_SET finalFillPolys = *fillInfo.m_Zone->Outline();
+
+            finalFillPolys.ClearArcs();
+            fillInfo.m_CombinedFill.ClearArcs();
+
+            // Intersect the zone outline with the combined fill that was assembled
+            // from all the related objects.
+            finalFillPolys.BooleanIntersection( fillInfo.m_CombinedFill );
+            finalFillPolys.Fracture( m_simplify );
+
+            // This is already mutex-ed, so this is safe
+            fillInfo.m_Zone->SetFilledPolysList( fillInfo.m_Layer, finalFillPolys );
+            return 1;
+        }
+
+        bool m_simplify;
+    };
+
+    /**
+     * Process the polygons in a thread pool for more fans, more faster
+     */
+    void ProcessPolygons( bool aSimplify )
+    {
+        PROF_TIMER timer( "Zone fill processing" );
+
+        COMPLEX_FIRST_FILL_TASK fillTask( aSimplify );
+        fillTask.Execute( m_FillInfos );
+
+        wxLogTrace( traceAllegroPerf, wxT( "   Intersected and fractured zone fills in %.3f ms" ), timer.msecs() ); // format:allow
+    }
+
+    void QueuePolygonForZone( ZONE& aZone, SHAPE_POLY_SET aFilledArea, PCB_LAYER_ID aLayer )
+    {
+        m_FillInfos.emplace_back( &aZone, aLayer, std::move( aFilledArea ) );
+    }
+
+private:
+    std::vector<FILL_INFO> m_FillInfos;
+};
+
+
 std::unique_ptr<ZONE> BOARD_BUILDER::buildZone( const BLOCK_BASE&                     aBoundaryBlock,
-                                                const std::vector<const BLOCK_BASE*>& aRelatedBlocks )
+                                                const std::vector<const BLOCK_BASE*>& aRelatedBlocks,
+                                                ZONE_FILL_HANDLER&                    aZoneFillHandler )
 {
     int              netCode = NETINFO_LIST::UNCONNECTED;
     const LAYER_INFO layerInfo = expectLayerFromBlock( aBoundaryBlock );
@@ -3927,7 +4019,7 @@ std::unique_ptr<ZONE> BOARD_BUILDER::buildZone( const BLOCK_BASE&
         {
         case 0x1B:
         {
-            auto it = m_netCache.find( block->GetKey() );
+            const auto it = m_netCache.find( block->GetKey() );
 
             if( it != m_netCache.end() )
             {
@@ -3950,7 +4042,6 @@ std::unique_ptr<ZONE> BOARD_BUILDER::buildZone( const BLOCK_BASE&
 
             SHAPE_POLY_SET fillPolySet = shapeToPolySet( shapeData );
             combinedFill.Append( fillPolySet );
-
             m_usedZoneFillShapes.emplace( block->GetKey() );
             break;
         }
@@ -3968,18 +4059,23 @@ std::unique_ptr<ZONE> BOARD_BUILDER::buildZone( const BLOCK_BASE&
     // Add zone fills
     if( isCopperZone && !combinedFill.IsEmpty() )
     {
-        SHAPE_POLY_SET zoneOutline = *zone->Outline();
+        // We don't do this here, though it feels like we should. We collect the
+        // information for batch processing later on (which is conceptually
+        // easier to parallelise compared to this function).  But there is room
+        // for improvement by threading more of this work: shapeToPolySet
+        // accounts for about 40% of the remaining single-threaded time in the
+        // building process.
 
-        combinedFill.ClearArcs();
-        zoneOutline.ClearArcs();
-
-        combinedFill.BooleanIntersection( zoneOutline );
-        combinedFill.Fracture( true );
-
-        zone->SetFilledPolysList( layer, combinedFill );
+        // combinedFill.ClearArcs();
+        // zoneOutline.ClearArcs();
+        // combinedFill.BooleanIntersection( zoneOutline );
+        // zone->SetFilledPolysList( layer, combinedFill );
 
         zone->SetIsFilled( true );
         zone->SetNeedRefill( false );
+
+        // Poke these relevant context in here for batch processing later on
+        aZoneFillHandler.QueuePolygonForZone( *zone, std::move( combinedFill ), layer );
     }
 
     return zone;
@@ -4081,6 +4177,8 @@ void BOARD_BUILDER::createZones()
     std::vector<std::unique_ptr<ZONE>> boundaryZones;
     std::vector<std::unique_ptr<ZONE>> keepoutZones;
 
+    ZONE_FILL_HANDLER zoneFillHandler;
+
     // Walk m_LL_Shapes to find BOUNDARY shapes (zone outlines).
     // BOUNDARY shapes use class 0x15 with copper layer subclass indices.
     const LL_WALKER shapeWalker( m_brdDb.m_Header->m_LL_Shapes, m_brdDb );
@@ -4095,7 +4193,7 @@ void BOARD_BUILDER::createZones()
         if( shapeData.m_Layer.m_Class != LAYER_INFO::CLASS::BOUNDARY )
             continue;
 
-        std::unique_ptr<ZONE> zone = buildZone( *block, getShapeRelatedBlocks( shapeData ) );
+        std::unique_ptr<ZONE> zone = buildZone( *block, getShapeRelatedBlocks( shapeData ), zoneFillHandler );
 
         if( zone )
         {
@@ -4127,7 +4225,7 @@ void BOARD_BUILDER::createZones()
             wxLogTrace( traceAllegroBuilder, "  Processing %s rect %#010x", layerInfoDisplayName( rectData.m_Layer ),
                         rectData.m_Key );
 
-            zone = buildZone( *block, {} );
+            zone = buildZone( *block, {}, zoneFillHandler );
             break;
         }
         case 0x28:
@@ -4140,7 +4238,7 @@ void BOARD_BUILDER::createZones()
             wxLogTrace( traceAllegroBuilder, "  Processing %s shape %#010x", layerInfoDisplayName( shapeData.m_Layer ),
                         shapeData.m_Key );
 
-            zone = buildZone( *block, {} );
+            zone = buildZone( *block, {}, zoneFillHandler );
             break;
         }
         default:
@@ -4153,6 +4251,9 @@ void BOARD_BUILDER::createZones()
         }
     }
 
+    // Deal with all the collected zone fill polygons now, all at once
+    zoneFillHandler.ProcessPolygons( true );
+
     int keepoutCount = keepoutZones.size();
     int boundaryCount = boundaryZones.size();
 

pcbnew/pcb_io/allegro/allegro_builder.h

@@ -163,13 +163,17 @@ private:
     std::vector<std::unique_ptr<BOARD_ITEM>> buildTrack( const BLK_0x05_TRACK& aBlock, int aNetcode );
     std::unique_ptr<BOARD_ITEM>              buildVia( const BLK_0x33_VIA& aBlock, int aNetcode );
 
+    class ZONE_FILL_HANDLER;
+
     /**
      * Build a ZONE from an 0x0E, 0x24 or 0x28 block.
-     * 
+     *
      * @param aRelatedBlocks are blocks to get net (0x1B) and fill (0x28) info from
+     * @param aZoneFillHandler is a management object for efficiently dealing with filled zones
      */
     std::unique_ptr<ZONE> buildZone( const BLOCK_BASE&                     aBoundaryBlock,
-                                     const std::vector<const BLOCK_BASE*>& aRelatedBlocks );
+                                     const std::vector<const BLOCK_BASE*>& aRelatedBlocks,
+                                     ZONE_FILL_HANDLER&                    aZoneFillHandler );
 
     SHAPE_LINE_CHAIN buildOutline( const BLK_0x0E_RECT& aRect ) const;
     SHAPE_LINE_CHAIN buildOutline( const BLK_0x24_RECT& aRect ) const;