Break up buildConnectionGraph for debugging

Needed to find bottlenecks in fns, so break out individual sections of
the massive function for easier understanding.

buildItemSubgraphs (one section of the previous function) would build
millions of connections that were never used as stacked pins created X!
connections.  Also tested using sets instead of lists and keeping unique
lists to avoid flagging but none of these were as performant as using
flags to remember which items had already been processed.

Fixes https://gitlab.com/kicad/code/kicad/issues/10974

(cherry picked from commit 17b1b68ac7)
This commit is contained in:
Seth Hillbrand 2022-03-11 13:46:21 -08:00
parent 7ab4d67e94
commit 5b5c7d41b4
4 changed files with 126 additions and 36 deletions

View File

@ -708,19 +708,7 @@ void CONNECTION_GRAPH::updateItemConnectivity( const SCH_SHEET_PATH& aSheet,
} }
// TODO(JE) This won't give the same subgraph IDs (and eventually net/graph codes) void CONNECTION_GRAPH::buildItemSubGraphs()
// to the same subgraph necessarily if it runs over and over again on the same
// sheet. We need:
//
// a) a cache of net/bus codes, like used before
// b) to persist the CONNECTION_GRAPH globally so the cache is persistent,
// c) some way of trying to avoid changing net names. so we should keep track
// of the previous driver of a net, and if it comes down to choosing between
// equally-prioritized drivers, choose the one that already exists as a driver
// on some portion of the items.
void CONNECTION_GRAPH::buildConnectionGraph()
{ {
// Recache all bus aliases for later use // Recache all bus aliases for later use
wxCHECK_RET( m_schematic, "Connection graph cannot be built without schematic pointer" ); wxCHECK_RET( m_schematic, "Connection graph cannot be built without schematic pointer" );
@ -754,21 +742,25 @@ void CONNECTION_GRAPH::buildConnectionGraph()
connection->SetSubgraphCode( subgraph->m_code ); connection->SetSubgraphCode( subgraph->m_code );
m_item_to_subgraph_map[item] = subgraph; m_item_to_subgraph_map[item] = subgraph;
std::list<SCH_ITEM*> members; std::list<SCH_ITEM*> memberlist;
auto get_items = auto get_items =
[&]( SCH_ITEM* aItem ) -> bool [&]( SCH_ITEM* aItem ) -> bool
{ {
SCH_CONNECTION* conn = aItem->GetOrInitConnection( sheet, this ); SCH_CONNECTION* conn = aItem->GetOrInitConnection( sheet, this );
bool unique = !( aItem->GetFlags() & CANDIDATE );
return ( conn->SubgraphCode() == 0 ); aItem->SetFlags( CANDIDATE );
return ( unique && conn && ( conn->SubgraphCode() == 0 ) );
}; };
std::copy_if( item->ConnectedItems( sheet ).begin(), std::copy_if( item->ConnectedItems( sheet ).begin(),
item->ConnectedItems( sheet ).end(), item->ConnectedItems( sheet ).end(),
std::back_inserter( members ), get_items ); std::back_inserter( memberlist ), get_items );
for( SCH_ITEM* connected_item : members ) for( SCH_ITEM* connected_item : memberlist )
{ {
if( connected_item->Type() == SCH_NO_CONNECT_T ) if( connected_item->Type() == SCH_NO_CONNECT_T )
subgraph->m_no_connect = connected_item; subgraph->m_no_connect = connected_item;
@ -782,24 +774,26 @@ void CONNECTION_GRAPH::buildConnectionGraph()
connected_conn->SetSubgraphCode( subgraph->m_code ); connected_conn->SetSubgraphCode( subgraph->m_code );
m_item_to_subgraph_map[connected_item] = subgraph; m_item_to_subgraph_map[connected_item] = subgraph;
subgraph->AddItem( connected_item ); subgraph->AddItem( connected_item );
SCH_ITEM_SET citemset = connected_item->ConnectedItems( sheet );
std::copy_if( connected_item->ConnectedItems( sheet ).begin(), std::copy_if( citemset.begin(), citemset.end(),
connected_item->ConnectedItems( sheet ).end(), std::back_inserter( memberlist ), get_items );
std::back_inserter( members ), get_items );
} }
} }
for( SCH_ITEM* connected_item : memberlist )
connected_item->ClearFlags( CANDIDATE );
subgraph->m_dirty = true; subgraph->m_dirty = true;
m_subgraphs.push_back( subgraph ); m_subgraphs.push_back( subgraph );
} }
} }
} }
/** }
* TODO(JE): Net codes are non-deterministic. Fortunately, they are also not really used for
* anything. We should consider removing them entirely and just using net names everywhere.
*/
void CONNECTION_GRAPH::resolveAllDrivers()
{
// Resolve drivers for subgraphs and propagate connectivity info // Resolve drivers for subgraphs and propagate connectivity info
// We don't want to spin up a new thread for fewer than 8 nets (overhead costs) // We don't want to spin up a new thread for fewer than 8 nets (overhead costs)
@ -879,7 +873,11 @@ void CONNECTION_GRAPH::buildConnectionGraph()
{ {
return candidate->m_driver; return candidate->m_driver;
} ); } );
}
void CONNECTION_GRAPH::collectAllDriverValues()
{
// Check for subgraphs with the same net name but only weak drivers. // Check for subgraphs with the same net name but only weak drivers.
// For example, two wires that are both connected to hierarchical // For example, two wires that are both connected to hierarchical
// sheet pins that happen to have the same name, but are not the same. // sheet pins that happen to have the same name, but are not the same.
@ -934,7 +932,11 @@ void CONNECTION_GRAPH::buildConnectionGraph()
} }
} }
} }
}
void CONNECTION_GRAPH::generateInvisiblePinSubGraphs()
{
// Generate subgraphs for invisible power pins. These will be merged with other subgraphs // Generate subgraphs for invisible power pins. These will be merged with other subgraphs
// on the same sheet in the next loop. // on the same sheet in the next loop.
@ -954,7 +956,7 @@ void CONNECTION_GRAPH::buildConnectionGraph()
SCH_CONNECTION* connection = pin->GetOrInitConnection( sheet, this ); SCH_CONNECTION* connection = pin->GetOrInitConnection( sheet, this );
// If this pin already has a subgraph, don't need to process // If this pin already has a subgraph, don't need to process
if( connection->SubgraphCode() > 0 ) if( !connection || connection->SubgraphCode() > 0 )
continue; continue;
connection->SetName( pin->GetShownName() ); connection->SetName( pin->GetShownName() );
@ -991,7 +993,11 @@ void CONNECTION_GRAPH::buildConnectionGraph()
connection->SetSubgraphCode( subgraph->m_code ); connection->SetSubgraphCode( subgraph->m_code );
} }
}
void CONNECTION_GRAPH::processSubGraphs()
{
// Here we do all the local (sheet) processing of each subgraph, including assigning net // Here we do all the local (sheet) processing of each subgraph, including assigning net
// codes, merging subgraphs together that use label connections, etc. // codes, merging subgraphs together that use label connections, etc.
@ -1318,6 +1324,49 @@ void CONNECTION_GRAPH::buildConnectionGraph()
subgraph->m_driver_connection->Name() ); subgraph->m_driver_connection->Name() );
} }
}
// TODO(JE) This won't give the same subgraph IDs (and eventually net/graph codes)
// to the same subgraph necessarily if it runs over and over again on the same
// sheet. We need:
//
// a) a cache of net/bus codes, like used before
// b) to persist the CONNECTION_GRAPH globally so the cache is persistent,
// c) some way of trying to avoid changing net names. so we should keep track
// of the previous driver of a net, and if it comes down to choosing between
// equally-prioritized drivers, choose the one that already exists as a driver
// on some portion of the items.
void CONNECTION_GRAPH::buildConnectionGraph()
{
// Recache all bus aliases for later use
wxCHECK_RET( m_schematic, wxT( "Connection graph cannot be built without schematic pointer" ) );
SCH_SHEET_LIST all_sheets = m_schematic->GetSheets();
for( unsigned i = 0; i < all_sheets.size(); i++ )
{
for( const auto& alias : all_sheets[i].LastScreen()->GetBusAliases() )
m_bus_alias_cache[ alias->GetName() ] = alias;
}
buildItemSubGraphs();
/**
* TODO(JE): Net codes are non-deterministic. Fortunately, they are also not really used for
* anything. We should consider removing them entirely and just using net names everywhere.
*/
resolveAllDrivers();
collectAllDriverValues();
generateInvisiblePinSubGraphs();
processSubGraphs();
// Absorbed subgraphs should no longer be considered // Absorbed subgraphs should no longer be considered
alg::delete_if( m_driver_subgraphs, [&]( const CONNECTION_SUBGRAPH* candidate ) -> bool alg::delete_if( m_driver_subgraphs, [&]( const CONNECTION_SUBGRAPH* candidate ) -> bool
{ {
@ -1340,7 +1389,13 @@ void CONNECTION_GRAPH::buildConnectionGraph()
m_sheet_to_subgraphs_map[ subgraph->m_sheet ].emplace_back( subgraph ); m_sheet_to_subgraphs_map[ subgraph->m_sheet ].emplace_back( subgraph );
// Update item connections at this point so that neighbor propagation works // Update item connections at this point so that neighbor propagation works
nextSubgraph.store( 0 ); std::atomic<size_t> nextSubgraph( 0 );
// We don't want to spin up a new thread for fewer than 8 nets (overhead costs)
size_t parallelThreadCount = std::min<size_t>( std::thread::hardware_concurrency(),
( m_subgraphs.size() + 3 ) / 4 );
std::vector<std::future<size_t>> returns( parallelThreadCount );
auto preliminaryUpdateTask = auto preliminaryUpdateTask =
[&]() -> size_t [&]() -> size_t

View File

@ -370,6 +370,33 @@ private:
*/ */
void buildConnectionGraph(); void buildConnectionGraph();
/**
* Generates individual item subgraphs on a per-sheet basis
*/
void buildItemSubGraphs();
/**
* Finds all subgraphs in the connection graph and calls ResolveDrivers() in
* parallel
*/
void resolveAllDrivers();
/**
* Maps the driver values for each subgraph
*/
void collectAllDriverValues();
/**
* Iterate through the invisible power pins to collect the global labels
* as drivers
*/
void generateInvisiblePinSubGraphs();
/**
* Process all subgraphs to assign netcodes and merge subgraphs based on labels
*/
void processSubGraphs();
/** /**
* Helper to assign a new net code to a connection * Helper to assign a new net code to a connection
* *

View File

@ -183,11 +183,13 @@ SCH_ITEM_SET& SCH_ITEM::ConnectedItems( const SCH_SHEET_PATH& aSheet )
void SCH_ITEM::AddConnectionTo( const SCH_SHEET_PATH& aSheet, SCH_ITEM* aItem ) void SCH_ITEM::AddConnectionTo( const SCH_SHEET_PATH& aSheet, SCH_ITEM* aItem )
{ {
// The vector elements are small, so reserve 1k at a time to prevent re-allocations SCH_ITEM_SET& set = m_connected_items[ aSheet ];
if( m_connected_items[ aSheet ].capacity() == 0 )
m_connected_items[ aSheet ].reserve( 1024 );
m_connected_items[ aSheet ].emplace_back( aItem ); // The vector elements are small, so reserve 1k at a time to prevent re-allocations
if( set.size() == set.capacity() )
set.reserve( set.size() + 4096 );
set.emplace_back( aItem );
} }
@ -217,6 +219,9 @@ SCH_CONNECTION* SCH_ITEM::InitializeConnection( const SCH_SHEET_PATH& aSheet,
SCH_CONNECTION* SCH_ITEM::GetOrInitConnection( const SCH_SHEET_PATH& aSheet, SCH_CONNECTION* SCH_ITEM::GetOrInitConnection( const SCH_SHEET_PATH& aSheet,
CONNECTION_GRAPH* aGraph ) CONNECTION_GRAPH* aGraph )
{ {
if( !IsConnectable() )
return nullptr;
SetConnectivityDirty( false ); SetConnectivityDirty( false );
SCH_CONNECTION* connection = Connection( &aSheet ); SCH_CONNECTION* connection = Connection( &aSheet );

View File

@ -217,12 +217,17 @@ bool SCH_LABEL_BASE::IsType( const KICAD_T aScanTypes[] ) const
{ {
if( *p == SCH_LABEL_LOCATE_ANY_T ) if( *p == SCH_LABEL_LOCATE_ANY_T )
return true; return true;
}
wxCHECK_MSG( Schematic(), false, wxT( "No parent SCHEMATIC set for SCH_LABEL!" ) );
const SCH_ITEM_SET& item_set = m_connected_items.at( Schematic()->CurrentSheet() );
for( const KICAD_T* p = aScanTypes; *p != EOT; ++p )
{
if( *p == SCH_LABEL_LOCATE_WIRE_T ) if( *p == SCH_LABEL_LOCATE_WIRE_T )
{ {
wxCHECK_MSG( Schematic(), false, "No parent SCHEMATIC set for SCH_LABEL!" ); for( SCH_ITEM* connection : item_set )
for( SCH_ITEM* connection : m_connected_items.at( Schematic()->CurrentSheet() ) )
{ {
if( connection->IsType( wireTypes ) ) if( connection->IsType( wireTypes ) )
return true; return true;
@ -231,9 +236,7 @@ bool SCH_LABEL_BASE::IsType( const KICAD_T aScanTypes[] ) const
if ( *p == SCH_LABEL_LOCATE_BUS_T ) if ( *p == SCH_LABEL_LOCATE_BUS_T )
{ {
wxCHECK_MSG( Schematic(), false, "No parent SCHEMATIC set for SCH_LABEL!" ); for( SCH_ITEM* connection : item_set )
for( SCH_ITEM* connection : m_connected_items.at( Schematic()->CurrentSheet() ) )
{ {
if( connection->IsType( busTypes ) ) if( connection->IsType( busTypes ) )
return true; return true;