haskal
/
kicad
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

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.

(cherry picked from commit 758974f5aa)
This commit is contained in:
Jeff Young 2024-06-06 11:40:24 +01:00
parent 02ca96df48
commit d304a30155
5 changed files with 57 additions and 73 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

28
eeschema/sch_sheet.cpp
View File

@ -1306,25 +1306,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 ) for( const SCH_SHEET_INSTANCE& instance : m_instances )
{ {
// if aSheetPath is found, nothing to do: // if aSheetPath is found, nothing to do:
if( instance.m_Path == aSheetPath.Path() ) if( instance.m_Path == aPath )
return false; return false;
} }
wxLogTrace( traceSchSheetPaths, wxT( "Adding instance `%s` to sheet `%s`." ), wxLogTrace( traceSchSheetPaths, wxT( "Adding instance `%s` to sheet `%s`." ),
aSheetPath.Path().AsString(), aPath.AsString(),
( GetName().IsEmpty() ) ? wxString( wxT( "root" ) ) : GetName() ); ( GetName().IsEmpty() ) ? wxString( wxT( "root" ) ) : GetName() );
SCH_SHEET_INSTANCE instance; 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. // This entry does not exist: add it with an empty page number.
m_instances.emplace_back( instance ); m_instances.emplace_back( instance );
@ -1384,17 +1381,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; wxString pageNumber;
KIID_PATH path = aSheetPath.Path();
for( const SCH_SHEET_INSTANCE& instance : m_instances ) for( const SCH_SHEET_INSTANCE& instance : m_instances )
{ {
if( instance.m_Path == path ) if( instance.m_Path == aPath )
{ {
pageNumber = instance.m_PageNumber; pageNumber = instance.m_PageNumber;
break; break;
@ -1405,16 +1398,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 ) for( SCH_SHEET_INSTANCE& instance : m_instances )
{ {
if( instance.m_Path == path ) if( instance.m_Path == aPath )
{ {
instance.m_PageNumber = aPageNumber; instance.m_PageNumber = aPageNumber;
break; break;

24
eeschema/sch_sheet.h
View File

@ -449,40 +449,34 @@ protected:
* in the list, do nothing. Sheet instances allow for the sharing in complex hierarchies * 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. * 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 * @warning The #KIID_PATH object must be a full hierarchical path which means the sheet
* #SCH_SHEET object at index 0 must be the root sheet. A partial sheet path * at index 0 must be the root sheet.
* will raise an assertion on debug builds and silently fail and return false
* on release builds.
* *
* @param[in] aInstance is the #SCH_SHEET_PATH of the sheet instance to the instance list. * @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. * @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. * Return the sheet page number for \a aInstance.
* *
* @warning The #SCH_SHEET_PATH object must be a full hierarchical path which means the * @warning The #KIID_PATH object must be a full hierarchical path which means the sheet
* #SCH_SHEET object at index 0 must be the root sheet. A partial sheet path * at index 0 must be the root sheet.
* will raise an assertion on debug builds and silently fail and return an empty
* page number on release builds.
* *
* @return the page number for the requested sheet instance. * @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. * 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 * @warning The #KIID_PATH object must be a full hierarchical path which means the sheet
* #SCH_SHEET object at index 0 must be the root sheet. A partial sheet path * at index 0 must be the root sheet.
* will raise an assertion on debug builds and silently fail and return on release
* builds.
* *
* @param[in] aInstance is the hierarchical path of the sheet. * @param[in] aInstance is the hierarchical path of the sheet.
* @param[in] aReference is the new page number for 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 getInstance( SCH_SHEET_INSTANCE& aInstance, const KIID_PATH& aSheetPath,
bool aTestFromEnd = false ) const; bool aTestFromEnd = false ) const;

39
eeschema/sch_sheet_path.cpp
View File

@ -149,9 +149,10 @@ SCH_SHEET_PATH SCH_SHEET_PATH::operator+( const SCH_SHEET_PATH& aOther )
void SCH_SHEET_PATH::initFromOther( const SCH_SHEET_PATH& aOther ) void SCH_SHEET_PATH::initFromOther( const SCH_SHEET_PATH& aOther )
{ {
m_sheets = aOther.m_sheets; m_sheets = aOther.m_sheets;
m_virtualPageNumber = aOther.m_virtualPageNumber; m_virtualPageNumber = aOther.m_virtualPageNumber;
m_current_hash = aOther.m_current_hash; 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 SCH_SHEET_PATHS to be // Note: don't copy m_recursion_test_cache as it is slow and we want SCH_SHEET_PATHS to be
// very fast to construct for use in the connectivity algorithm. // very fast to construct for use in the connectivity algorithm.
@ -290,6 +291,7 @@ wxString SCH_SHEET_PATH::PathAsString() const
KIID_PATH SCH_SHEET_PATH::Path() const KIID_PATH SCH_SHEET_PATH::Path() const
{ {
KIID_PATH path; KIID_PATH path;
path.reserve( m_sheets.size() );
for( const SCH_SHEET* sheet : m_sheets ) for( const SCH_SHEET* sheet : m_sheets )
path.push_back( sheet->m_Uuid ); path.push_back( sheet->m_Uuid );
@ -537,8 +539,7 @@ wxString SCH_SHEET_PATH::GetPageNumber() const
wxCHECK( sheet, wxEmptyString ); wxCHECK( sheet, wxEmptyString );
SCH_SHEET_PATH tmpPath = *this; KIID_PATH tmpPath = Path();
tmpPath.pop_back(); tmpPath.pop_back();
return sheet->getPageNumber( tmpPath ); return sheet->getPageNumber( tmpPath );
@ -551,7 +552,7 @@ void SCH_SHEET_PATH::SetPageNumber( const wxString& aPageNumber )
wxCHECK( sheet, /* void */ ); wxCHECK( sheet, /* void */ );
SCH_SHEET_PATH tmpPath = *this; KIID_PATH tmpPath = Path();
tmpPath.pop_back(); tmpPath.pop_back();
@ -739,17 +740,27 @@ void SCH_SHEET_LIST::BuildSheetList( SCH_SHEET* aSheet, bool aCheckIntegrity )
void SCH_SHEET_LIST::SortByPageNumbers( bool aUpdateVirtualPageNums ) void SCH_SHEET_LIST::SortByPageNumbers( bool aUpdateVirtualPageNums )
{ {
for( const SCH_SHEET_PATH& path : *this )
path.CachePageNumber();
std::sort( begin(), end(), 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 ) if( retval < 0 )
return true; return true;
else if( retval > 0 ) else if( retval > 0 )
return false; 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 ) if( aUpdateVirtualPageNums )
@ -757,9 +768,7 @@ void SCH_SHEET_LIST::SortByPageNumbers( bool aUpdateVirtualPageNums )
int virtualPageNum = 1; int virtualPageNum = 1;
for( SCH_SHEET_PATH& sheet : *this ) for( SCH_SHEET_PATH& sheet : *this )
{
sheet.SetVirtualPageNumber( virtualPageNum++ ); sheet.SetVirtualPageNumber( virtualPageNum++ );
}
} }
} }
@ -1284,7 +1293,7 @@ void SCH_SHEET_LIST::AddNewSheetInstances( const SCH_SHEET_PATH& aPrefixSheetPat
for( SCH_SHEET_PATH& sheetPath : *this ) for( SCH_SHEET_PATH& sheetPath : *this )
{ {
SCH_SHEET_PATH tmp( sheetPath ); KIID_PATH tmp = sheetPath.Path();
SCH_SHEET_PATH newSheetPath( aPrefixSheetPath ); SCH_SHEET_PATH newSheetPath( aPrefixSheetPath );
// Prefix the new hierarchical path. // Prefix the new hierarchical path.
@ -1302,10 +1311,10 @@ void SCH_SHEET_LIST::AddNewSheetInstances( const SCH_SHEET_PATH& aPrefixSheetPat
SCH_SHEET_INSTANCE instance; SCH_SHEET_INSTANCE instance;
// Add the instance if it doesn't already exist // 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->addInstance( tmp );
sheet->getInstance( instance, tmp.Path(), true ); sheet->getInstance( instance, tmp, true );
} }
// Get a new page number if we don't have one // Get a new page number if we don't have one

8
eeschema/sch_sheet_path.h
View File

@ -233,6 +233,9 @@ public:
*/ */
int Cmp( const SCH_SHEET_PATH& aSheetPathToTest ) const; 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 * Compare sheets by their page number. If the actual page number is equal, use virtual page
* numbers to compare. * numbers to compare.
@ -415,9 +418,10 @@ private:
void initFromOther( const SCH_SHEET_PATH& aOther ); void initFromOther( const SCH_SHEET_PATH& aOther );
protected: 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. int m_virtualPageNumber; /// Page numbers are maintained by the sheet load order.

31
eeschema/tools/sch_find_replace_tool.cpp
View File

@ -253,50 +253,39 @@ int SCH_FIND_REPLACE_TOOL::FindNext( const TOOL_EVENT& aEvent )
if( !item && searchAllSheets ) if( !item && searchAllSheets )
{ {
SCH_SCREENS screens( m_frame->Schematic().Root() ); SCH_SCREENS screens( m_frame->Schematic().Root() );
std::vector<SCH_SHEET_PATH*> paths; SCH_SHEET_LIST paths;
screens.BuildClientSheetPathList(); screens.BuildClientSheetPathList();
for( SCH_SCREEN* screen = screens.GetFirst(); screen; screen = screens.GetNext() ) for( SCH_SCREEN* screen = screens.GetFirst(); screen; screen = screens.GetNext() )
{ {
for( SCH_SHEET_PATH& sheet : screen->GetClientSheetPaths() ) 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, paths.SortByPageNumbers( false );
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();
} );
if( isReversed ) if( isReversed )
std::reverse( paths.begin(), paths.end() ); std::reverse( paths.begin(), paths.end() );
for( SCH_SHEET_PATH* sheet : paths ) for( SCH_SHEET_PATH& sheet : paths )
{ {
if( afterSheet ) if( afterSheet )
{ {
if( afterSheet->GetCurrentHash() == sheet->GetCurrentHash() ) if( afterSheet->GetCurrentHash() == sheet.GetCurrentHash() )
afterSheet = nullptr; afterSheet = nullptr;
continue; continue;
} }
item = nextMatch( sheet->LastScreen(), sheet, nullptr, data, isReversed ); item = nextMatch( sheet.LastScreen(), &sheet, nullptr, data, isReversed );
if( item ) 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(); m_frame->DisplayCurrentSheet();
} }
@ -442,7 +431,7 @@ int SCH_FIND_REPLACE_TOOL::ReplaceAll( const TOOL_EVENT& aEvent )
} }
else else
{ {
SCH_SHEET_LIST allSheets = m_frame->Schematic().GetSheets(); SCH_SHEET_LIST allSheets = m_frame->Schematic().GetUnorderedSheets();
SCH_SCREENS screens( m_frame->Schematic().Root() ); SCH_SCREENS screens( m_frame->Schematic().Root() );
for( SCH_SCREEN* screen = screens.GetFirst(); screen; screen = screens.GetNext() ) for( SCH_SCREEN* screen = screens.GetFirst(); screen; screen = screens.GetNext() )