Performance for large hierarchies: sorting

Cache page numbers during sort.
Don't construct SCH_SHEET_PATH when reference will do.
Don't construct SCH_SHEET_PATH when KIID_PATH will do.
This commit is contained in:
Jeff Young 2024-06-06 11:40:24 +01:00
parent a6e8cfe35f
commit 758974f5aa
5 changed files with 57 additions and 73 deletions

View File

@ -1426,25 +1426,22 @@ void SCH_SHEET::AddInstance( const SCH_SHEET_INSTANCE& aInstance )
}
bool SCH_SHEET::addInstance( const SCH_SHEET_PATH& aSheetPath )
bool SCH_SHEET::addInstance( const KIID_PATH& aPath )
{
wxCHECK( aSheetPath.IsFullPath(), false );
wxCHECK( !aSheetPath.Last() || ( aSheetPath.Last()->m_Uuid != m_Uuid ), false );
for( const SCH_SHEET_INSTANCE& instance : m_instances )
{
// if aSheetPath is found, nothing to do:
if( instance.m_Path == aSheetPath.Path() )
if( instance.m_Path == aPath )
return false;
}
wxLogTrace( traceSchSheetPaths, wxT( "Adding instance `%s` to sheet `%s`." ),
aSheetPath.Path().AsString(),
aPath.AsString(),
( GetName().IsEmpty() ) ? wxString( wxT( "root" ) ) : GetName() );
SCH_SHEET_INSTANCE instance;
instance.m_Path = aSheetPath.Path();
instance.m_Path = aPath;
// This entry does not exist: add it with an empty page number.
m_instances.emplace_back( instance );
@ -1504,17 +1501,13 @@ const SCH_SHEET_INSTANCE& SCH_SHEET::GetRootInstance() const
}
wxString SCH_SHEET::getPageNumber( const SCH_SHEET_PATH& aSheetPath ) const
wxString SCH_SHEET::getPageNumber( const KIID_PATH& aPath ) const
{
wxCHECK( aSheetPath.IsFullPath(), wxEmptyString );
wxCHECK( !aSheetPath.Last() || ( aSheetPath.Last()->m_Uuid != m_Uuid ), wxEmptyString );
wxString pageNumber;
KIID_PATH path = aSheetPath.Path();
for( const SCH_SHEET_INSTANCE& instance : m_instances )
{
if( instance.m_Path == path )
if( instance.m_Path == aPath )
{
pageNumber = instance.m_PageNumber;
break;
@ -1525,16 +1518,11 @@ wxString SCH_SHEET::getPageNumber( const SCH_SHEET_PATH& aSheetPath ) const
}
void SCH_SHEET::setPageNumber( const SCH_SHEET_PATH& aSheetPath, const wxString& aPageNumber )
void SCH_SHEET::setPageNumber( const KIID_PATH& aPath, const wxString& aPageNumber )
{
wxCHECK( aSheetPath.IsFullPath(), /* void */ );
wxCHECK( !aSheetPath.Last() || ( aSheetPath.Last()->m_Uuid != m_Uuid ), /* void */ );
KIID_PATH path = aSheetPath.Path();
for( SCH_SHEET_INSTANCE& instance : m_instances )
{
if( instance.m_Path == path )
if( instance.m_Path == aPath )
{
instance.m_PageNumber = aPageNumber;
break;

View File

@ -474,40 +474,34 @@ protected:
* in the list, do nothing. Sheet instances allow for the sharing in complex hierarchies
* which allows for per instance data such as page number for sheets to stored.
*
* @warning The #SCH_SHEET_PATH object must be a full hierarchical path which means the
* #SCH_SHEET object at index 0 must be the root sheet. A partial sheet path
* will raise an assertion on debug builds and silently fail and return false
* on release builds.
* @warning The #KIID_PATH object must be a full hierarchical path which means the sheet
* at index 0 must be the root sheet.
*
* @param[in] aInstance is the #SCH_SHEET_PATH of the sheet instance to the instance list.
* @return false if the instance already exists, true if the instance was added.
*/
bool addInstance( const SCH_SHEET_PATH& aInstance );
bool addInstance( const KIID_PATH& aInstance );
/**
* Return the sheet page number for \a aInstance.
*
* @warning The #SCH_SHEET_PATH object must be a full hierarchical path which means the
* #SCH_SHEET object at index 0 must be the root sheet. A partial sheet path
* will raise an assertion on debug builds and silently fail and return an empty
* page number on release builds.
* @warning The #KIID_PATH object must be a full hierarchical path which means the sheet
* at index 0 must be the root sheet.
*
* @return the page number for the requested sheet instance.
*/
wxString getPageNumber( const SCH_SHEET_PATH& aInstance ) const;
wxString getPageNumber( const KIID_PATH& aInstance ) const;
/**
* Set the page number for the sheet instance \a aInstance.
*
* @warning The #SCH_SHEET_PATH object must be a full hierarchical path which means the
* #SCH_SHEET object at index 0 must be the root sheet. A partial sheet path
* will raise an assertion on debug builds and silently fail and return on release
* builds.
* @warning The #KIID_PATH object must be a full hierarchical path which means the sheet
* at index 0 must be the root sheet.
*
* @param[in] aInstance is the hierarchical path of the sheet.
* @param[in] aReference is the new page number for the sheet.
*/
void setPageNumber( const SCH_SHEET_PATH& aInstance, const wxString& aPageNumber );
void setPageNumber( const KIID_PATH& aInstance, const wxString& aPageNumber );
bool getInstance( SCH_SHEET_INSTANCE& aInstance, const KIID_PATH& aSheetPath,
bool aTestFromEnd = false ) const;

View File

@ -138,9 +138,10 @@ SCH_SHEET_PATH SCH_SHEET_PATH::operator+( const SCH_SHEET_PATH& aOther )
void SCH_SHEET_PATH::initFromOther( const SCH_SHEET_PATH& aOther )
{
m_sheets = aOther.m_sheets;
m_virtualPageNumber = aOther.m_virtualPageNumber;
m_current_hash = aOther.m_current_hash;
m_sheets = aOther.m_sheets;
m_virtualPageNumber = aOther.m_virtualPageNumber;
m_current_hash = aOther.m_current_hash;
m_cached_page_number = aOther.m_cached_page_number;
// Note: don't copy m_recursion_test_cache as it is slow and we want std::vector<SCH_SHEET_PATH> to be
// very fast to construct for use in the connectivity algorithm.
@ -327,6 +328,7 @@ wxString SCH_SHEET_PATH::PathAsString() const
KIID_PATH SCH_SHEET_PATH::Path() const
{
KIID_PATH path;
path.reserve( m_sheets.size() );
for( const SCH_SHEET* sheet : m_sheets )
path.push_back( sheet->m_Uuid );
@ -574,8 +576,7 @@ wxString SCH_SHEET_PATH::GetPageNumber() const
wxCHECK( sheet, wxEmptyString );
SCH_SHEET_PATH tmpPath = *this;
KIID_PATH tmpPath = Path();
tmpPath.pop_back();
return sheet->getPageNumber( tmpPath );
@ -588,7 +589,7 @@ void SCH_SHEET_PATH::SetPageNumber( const wxString& aPageNumber )
wxCHECK( sheet, /* void */ );
SCH_SHEET_PATH tmpPath = *this;
KIID_PATH tmpPath = Path();
tmpPath.pop_back();
@ -776,17 +777,27 @@ void SCH_SHEET_LIST::BuildSheetList( SCH_SHEET* aSheet, bool aCheckIntegrity )
void SCH_SHEET_LIST::SortByPageNumbers( bool aUpdateVirtualPageNums )
{
for( const SCH_SHEET_PATH& path : *this )
path.CachePageNumber();
std::sort( begin(), end(),
[]( SCH_SHEET_PATH a, SCH_SHEET_PATH b ) -> bool
[]( const SCH_SHEET_PATH& a, const SCH_SHEET_PATH& b ) -> bool
{
int retval = a.ComparePageNum( b );
int retval = SCH_SHEET::ComparePageNum( a.GetCachedPageNumber(),
b.GetCachedPageNumber() );
if( retval < 0 )
return true;
else if( retval > 0 )
return false;
else /// Enforce strict ordering. If the page numbers are the same, use UUIDs
return a.GetCurrentHash() < b.GetCurrentHash();
if( a.GetVirtualPageNumber() < b.GetVirtualPageNumber() )
return true;
else if( a.GetVirtualPageNumber() > b.GetVirtualPageNumber() )
return false;
/// Enforce strict ordering. If the page numbers are the same, use UUIDs
return a.GetCurrentHash() < b.GetCurrentHash();
} );
if( aUpdateVirtualPageNums )
@ -794,9 +805,7 @@ void SCH_SHEET_LIST::SortByPageNumbers( bool aUpdateVirtualPageNums )
int virtualPageNum = 1;
for( SCH_SHEET_PATH& sheet : *this )
{
sheet.SetVirtualPageNumber( virtualPageNum++ );
}
}
}
@ -1321,7 +1330,7 @@ void SCH_SHEET_LIST::AddNewSheetInstances( const SCH_SHEET_PATH& aPrefixSheetPat
for( SCH_SHEET_PATH& sheetPath : *this )
{
SCH_SHEET_PATH tmp( sheetPath );
KIID_PATH tmp = sheetPath.Path();
SCH_SHEET_PATH newSheetPath( aPrefixSheetPath );
// Prefix the new hierarchical path.
@ -1339,10 +1348,10 @@ void SCH_SHEET_LIST::AddNewSheetInstances( const SCH_SHEET_PATH& aPrefixSheetPat
SCH_SHEET_INSTANCE instance;
// Add the instance if it doesn't already exist
if( !sheet->getInstance( instance, tmp.Path(), true ) )
if( !sheet->getInstance( instance, tmp, true ) )
{
sheet->addInstance( tmp );
sheet->getInstance( instance, tmp.Path(), true );
sheet->getInstance( instance, tmp, true );
}
// Get a new page number if we don't have one

View File

@ -229,6 +229,9 @@ public:
*/
int Cmp( const SCH_SHEET_PATH& aSheetPathToTest ) const;
void CachePageNumber() const { m_cached_page_number = GetPageNumber(); }
wxString GetCachedPageNumber() const { return m_cached_page_number; }
/**
* Compare sheets by their page number. If the actual page number is equal, use virtual page
* numbers to compare.
@ -416,9 +419,10 @@ private:
void initFromOther( const SCH_SHEET_PATH& aOther );
protected:
std::vector< SCH_SHEET* > m_sheets;
std::vector<SCH_SHEET*> m_sheets;
size_t m_current_hash;
size_t m_current_hash;
mutable wxString m_cached_page_number;
int m_virtualPageNumber; /// Page numbers are maintained by the sheet load order.

View File

@ -253,50 +253,39 @@ int SCH_FIND_REPLACE_TOOL::FindNext( const TOOL_EVENT& aEvent )
if( !item && searchAllSheets )
{
SCH_SCREENS screens( m_frame->Schematic().Root() );
std::vector<SCH_SHEET_PATH*> paths;
SCH_SCREENS screens( m_frame->Schematic().Root() );
SCH_SHEET_LIST paths;
screens.BuildClientSheetPathList();
for( SCH_SCREEN* screen = screens.GetFirst(); screen; screen = screens.GetNext() )
{
for( SCH_SHEET_PATH& sheet : screen->GetClientSheetPaths() )
paths.push_back( &sheet );
paths.push_back( sheet );
}
std::sort( paths.begin(), paths.end(), [] ( const SCH_SHEET_PATH* lhs,
const SCH_SHEET_PATH* rhs ) -> bool
{
int retval = lhs->ComparePageNum( *rhs );
if( retval < 0 )
return true;
else if( retval > 0 )
return false;
else /// Enforce strict ordering. If the page numbers are the same, use UUIDs
return lhs->GetCurrentHash() < rhs->GetCurrentHash();
} );
paths.SortByPageNumbers( false );
if( isReversed )
std::reverse( paths.begin(), paths.end() );
for( SCH_SHEET_PATH* sheet : paths )
for( SCH_SHEET_PATH& sheet : paths )
{
if( afterSheet )
{
if( afterSheet->GetCurrentHash() == sheet->GetCurrentHash() )
if( afterSheet->GetCurrentHash() == sheet.GetCurrentHash() )
afterSheet = nullptr;
continue;
}
item = nextMatch( sheet->LastScreen(), sheet, nullptr, data, isReversed );
item = nextMatch( sheet.LastScreen(), &sheet, nullptr, data, isReversed );
if( item )
{
if( m_frame->Schematic().CurrentSheet() != *sheet )
if( m_frame->Schematic().CurrentSheet() != sheet )
{
m_frame->Schematic().SetCurrentSheet( *sheet );
m_frame->Schematic().SetCurrentSheet( sheet );
m_frame->DisplayCurrentSheet();
}
@ -442,7 +431,7 @@ int SCH_FIND_REPLACE_TOOL::ReplaceAll( const TOOL_EVENT& aEvent )
}
else
{
SCH_SHEET_LIST allSheets = m_frame->Schematic().GetSheets();
SCH_SHEET_LIST allSheets = m_frame->Schematic().GetUnorderedSheets();
SCH_SCREENS screens( m_frame->Schematic().Root() );
for( SCH_SCREEN* screen = screens.GetFirst(); screen; screen = screens.GetNext() )