From 1f1b97212bd6f2f014fc3cdfe3b1bd7db6006632 Mon Sep 17 00:00:00 2001 From: Armin Schoisswohl Date: Wed, 6 Mar 2024 15:26:30 +0100 Subject: [PATCH] change m_CachesMutex to shared_mutex and do shared locking for read access in zone BBox calculations --- pcbnew/board.cpp | 2 +- pcbnew/board.h | 4 ++-- pcbnew/drc/drc_cache_generator.cpp | 4 ++-- pcbnew/drc/drc_test_provider_disallow.cpp | 2 +- pcbnew/pcbexpr_evaluator.cpp | 12 ++++++------ pcbnew/pcbexpr_functions.cpp | 22 ++++++++++----------- pcbnew/zone.cpp | 24 +++++++++++++++++++---- 7 files changed, 43 insertions(+), 27 deletions(-) diff --git a/pcbnew/board.cpp b/pcbnew/board.cpp index fe7a7c3ce4..85dad68951 100644 --- a/pcbnew/board.cpp +++ b/pcbnew/board.cpp @@ -262,7 +262,7 @@ void BOARD::IncrementTimeStamp() || !m_ZoneBBoxCache.empty() || m_CopperItemRTreeCache ) { - std::unique_lock cacheLock( m_CachesMutex ); + std::unique_lock cacheLock( m_CachesMutex ); m_IntersectsAreaCache.clear(); m_EnclosedByAreaCache.clear(); diff --git a/pcbnew/board.h b/pcbnew/board.h index 93700cfd34..6e3a1a337e 100644 --- a/pcbnew/board.h +++ b/pcbnew/board.h @@ -35,7 +35,7 @@ #include #include #include -#include +#include #include class BOARD_DESIGN_SETTINGS; @@ -1227,7 +1227,7 @@ public: }; // ------------ Run-time caches ------------- - std::mutex m_CachesMutex; + std::shared_mutex m_CachesMutex; std::unordered_map m_IntersectsCourtyardCache; std::unordered_map m_IntersectsFCourtyardCache; std::unordered_map m_IntersectsBCourtyardCache; diff --git a/pcbnew/drc/drc_cache_generator.cpp b/pcbnew/drc/drc_cache_generator.cpp index 1b94c77d08..47d40851f6 100644 --- a/pcbnew/drc/drc_cache_generator.cpp +++ b/pcbnew/drc/drc_cache_generator.cpp @@ -136,7 +136,7 @@ bool DRC_CACHE_GENERATOR::Run() std::future retn = tp.submit( [&]() { - std::unique_lock cacheLock( m_board->m_CachesMutex ); + std::unique_lock cacheLock( m_board->m_CachesMutex ); if( !m_board->m_CopperItemRTreeCache ) m_board->m_CopperItemRTreeCache = std::make_shared(); @@ -184,7 +184,7 @@ bool DRC_CACHE_GENERATOR::Run() rtree->Insert( aZone, layer ); } - std::unique_lock cacheLock( m_board->m_CachesMutex ); + std::unique_lock cacheLock( m_board->m_CachesMutex ); m_board->m_CopperZoneRTreeCache[ aZone ] = std::move( rtree ); done.fetch_add( 1 ); diff --git a/pcbnew/drc/drc_test_provider_disallow.cpp b/pcbnew/drc/drc_test_provider_disallow.cpp index 5052022b0e..739e1f41dd 100644 --- a/pcbnew/drc/drc_test_provider_disallow.cpp +++ b/pcbnew/drc/drc_test_provider_disallow.cpp @@ -153,7 +153,7 @@ bool DRC_TEST_PROVIDER_DISALLOW::Run() PTR_PTR_LAYER_CACHE_KEY key = { ruleArea, copperZone, UNDEFINED_LAYER }; { - std::unique_lock cacheLock( board->m_CachesMutex ); + std::unique_lock cacheLock( board->m_CachesMutex ); board->m_IntersectsAreaCache[ key ] = isInside; } diff --git a/pcbnew/pcbexpr_evaluator.cpp b/pcbnew/pcbexpr_evaluator.cpp index fa1a6c3ec4..4621243b39 100644 --- a/pcbnew/pcbexpr_evaluator.cpp +++ b/pcbnew/pcbexpr_evaluator.cpp @@ -57,12 +57,12 @@ public: // in the ENUM_MAP: one for the canonical layer name and one for the user layer name. // We need to check against both. - wxPGChoices& layerMap = ENUM_MAP::Instance().Choices(); - const wxString& layerName = b->AsString(); - BOARD* board = static_cast( aCtx )->GetBoard(); - std::unique_lock cacheLock( board->m_CachesMutex ); - auto i = board->m_LayerExpressionCache.find( layerName ); - LSET mask; + wxPGChoices& layerMap = ENUM_MAP::Instance().Choices(); + const wxString& layerName = b->AsString(); + BOARD* board = static_cast( aCtx )->GetBoard(); + std::unique_lock cacheLock( board->m_CachesMutex ); + auto i = board->m_LayerExpressionCache.find( layerName ); + LSET mask; if( i == board->m_LayerExpressionCache.end() ) { diff --git a/pcbnew/pcbexpr_functions.cpp b/pcbnew/pcbexpr_functions.cpp index 9d82c175f9..36b355a4dd 100644 --- a/pcbnew/pcbexpr_functions.cpp +++ b/pcbnew/pcbexpr_functions.cpp @@ -135,7 +135,7 @@ static void existsOnLayerFunc( LIBEVAL::CONTEXT* aCtx, void *self ) */ BOARD* board = item->GetBoard(); - std::unique_lock cacheLock( board->m_CachesMutex ); + std::unique_lock cacheLock( board->m_CachesMutex ); auto i = board->m_LayerExpressionCache.find( layerName ); LSET mask; @@ -271,8 +271,8 @@ static void intersectsCourtyardFunc( LIBEVAL::CONTEXT* aCtx, void* self ) if( searchFootprints( board, arg->AsString(), context, [&]( FOOTPRINT* fp ) { - PTR_PTR_CACHE_KEY key = { fp, item }; - std::unique_lock cacheLock( board->m_CachesMutex ); + PTR_PTR_CACHE_KEY key = { fp, item }; + std::unique_lock cacheLock( board->m_CachesMutex ); if( ( item->GetFlags() & ROUTER_TRANSIENT ) == 0 ) { @@ -331,8 +331,8 @@ static void intersectsFrontCourtyardFunc( LIBEVAL::CONTEXT* aCtx, void* self ) if( searchFootprints( board, arg->AsString(), context, [&]( FOOTPRINT* fp ) { - PTR_PTR_CACHE_KEY key = { fp, item }; - std::unique_lock cacheLock( board->m_CachesMutex ); + PTR_PTR_CACHE_KEY key = { fp, item }; + std::unique_lock cacheLock( board->m_CachesMutex ); if( ( item->GetFlags() & ROUTER_TRANSIENT ) == 0 ) { @@ -390,8 +390,8 @@ static void intersectsBackCourtyardFunc( LIBEVAL::CONTEXT* aCtx, void* self ) if( searchFootprints( board, arg->AsString(), context, [&]( FOOTPRINT* fp ) { - PTR_PTR_CACHE_KEY key = { fp, item }; - std::unique_lock cacheLock( board->m_CachesMutex ); + PTR_PTR_CACHE_KEY key = { fp, item }; + std::unique_lock cacheLock( board->m_CachesMutex ); if( ( item->GetFlags() & ROUTER_TRANSIENT ) == 0 ) { @@ -660,7 +660,7 @@ static void intersectsAreaFunc( LIBEVAL::CONTEXT* aCtx, void* self ) { if( ( item->GetFlags() & ROUTER_TRANSIENT ) == 0 ) { - std::unique_lock cacheLock( board->m_CachesMutex ); + std::shared_lock cacheLock( board->m_CachesMutex ); key = { aArea, item, layer }; auto i = board->m_IntersectsAreaCache.find( key ); @@ -673,7 +673,7 @@ static void intersectsAreaFunc( LIBEVAL::CONTEXT* aCtx, void* self ) if( ( item->GetFlags() & ROUTER_TRANSIENT ) == 0 ) { - std::unique_lock cacheLock( board->m_CachesMutex ); + std::unique_lock cacheLock( board->m_CachesMutex ); board->m_IntersectsAreaCache[ key ] = collides; } @@ -735,8 +735,8 @@ static void enclosedByAreaFunc( LIBEVAL::CONTEXT* aCtx, void* self ) if( !aArea->GetBoundingBox().Intersects( itemBBox ) ) return false; - std::unique_lock cacheLock( board->m_CachesMutex ); - PTR_PTR_LAYER_CACHE_KEY key = { aArea, item, layer }; + std::unique_lock cacheLock( board->m_CachesMutex ); + PTR_PTR_LAYER_CACHE_KEY key = { aArea, item, layer }; if( ( item->GetFlags() & ROUTER_TRANSIENT ) == 0 ) { diff --git a/pcbnew/zone.cpp b/pcbnew/zone.cpp index 11ff96c04c..2a9996d78c 100644 --- a/pcbnew/zone.cpp +++ b/pcbnew/zone.cpp @@ -344,13 +344,24 @@ const BOX2I ZONE::GetBoundingBox() const if( const BOARD* board = GetBoard() ) { std::unordered_map& cache = board->m_ZoneBBoxCache; - std::unique_lock cacheLock( const_cast( board )->m_CachesMutex ); + std::shared_lock readLock( const_cast( board )->m_CachesMutex ); auto cacheIter = cache.find( this ); + if( cacheIter != cache.end() ) + return cacheIter->second; + + readLock.unlock(); + + // if we get here we need an exclusive lock to cache the entry + std::unique_lock writeLock( const_cast( board )->m_CachesMutex ); + + // check again for the cached item; it might have been computed meanwhile by another thread + cacheIter = cache.find( this ); if( cacheIter != cache.end() ) return cacheIter->second; BOX2I bbox = m_Poly->BBox(); + cache[ this ] = bbox; return bbox; @@ -364,11 +375,16 @@ void ZONE::CacheBoundingBox() { BOARD* board = GetBoard(); std::unordered_map& cache = board->m_ZoneBBoxCache; - std::unique_lock cacheLock( board->m_CachesMutex ); + std::shared_lock readLock( board->m_CachesMutex ); auto cacheIter = cache.find( this ); - if( cacheIter == cache.end() ) - cache[ this ] = m_Poly->BBox(); + if( cacheIter == cache.end() ) { + readLock.unlock(); + std::unique_lock writeLock(board->m_CachesMutex); + // check again in case another thread has already calculated the entry while we were waiting for the exclusive lock + if ( cache.count( this ) == 0 ) + cache[this] = m_Poly->BBox(); + } }