From 916f79d5a1902201b022937a19519080d56c6a61 Mon Sep 17 00:00:00 2001 From: Jeff Young Date: Mon, 5 Feb 2018 20:55:00 +0000 Subject: [PATCH] Performance fixes for the place footprint list all dialog. Smartens the cache freshness checking, and adds checksums of constituent-directory last-mod-dates to the footprint info list. Also inserts the dataPtrs into the list at the same time as the text to keep wxWidgets from measuring the width of the text twice (yes, really). And converts the list to default- column widths in case the wxWidgets patch is present to greatly speed list creation (by not measuring all that text). On my machine drops the first-load time from 4s to 2.5s and the subsequent-load times from 2.5s to < 1s. With the wxWidgets patch subsequent-loads become near-instantaneous. --- common/displlst.cpp | 47 ++++++++++++------------ common/fp_lib_table.cpp | 22 ++++++++++++ include/fp_lib_table.h | 6 ++++ pcbnew/footprint_info_impl.cpp | 9 ++++- pcbnew/footprint_info_impl.h | 1 + pcbnew/io_mgr.h | 10 ++++++ pcbnew/kicad_plugin.cpp | 66 ++++++++-------------------------- pcbnew/kicad_plugin.h | 4 ++- 8 files changed, 88 insertions(+), 77 deletions(-) diff --git a/common/displlst.cpp b/common/displlst.cpp index 7e2eddda2d..bcd124089b 100644 --- a/common/displlst.cpp +++ b/common/displlst.cpp @@ -33,6 +33,15 @@ #include #include + +// wxWidgets spends *far* too long calcuating column widths (most of it, believe it or +// not, in repeatedly creating/destorying a wxDC to do the measurement in). +// Use default column widths instead. (Note that these will be scaled down proportionally +// to fit the available space when the dialog is instantiated.) +static int DEFAULT_COL_WIDTHS[] = { 400, 200 }; + + + EDA_LIST_DIALOG::EDA_LIST_DIALOG( EDA_DRAW_FRAME* aParent, const wxString& aTitle, const wxArrayString& aItemHeaders, const std::vector& aItemList, @@ -64,19 +73,11 @@ EDA_LIST_DIALOG::EDA_LIST_DIALOG( EDA_DRAW_FRAME* aParent, const wxString& aTitl } -void EDA_LIST_DIALOG::initDialog( const wxArrayString& aItemHeaders, - const wxString& aSelection) +void EDA_LIST_DIALOG::initDialog( const wxArrayString& aItemHeaders, const wxString& aSelection) { - for( unsigned i = 0; i < aItemHeaders.Count(); i++ ) - { - wxListItem column; - - column.SetId( i ); - column.SetText( aItemHeaders.Item( i ) ); - - m_listBox->InsertColumn( i, column ); - } + m_listBox->InsertColumn( i, aItemHeaders.Item( i ), + wxLIST_FORMAT_LEFT, DEFAULT_COL_WIDTHS[ i ] ); InsertItems( *m_itemsListCp, 0 ); @@ -86,15 +87,6 @@ void EDA_LIST_DIALOG::initDialog( const wxArrayString& aItemHeaders, m_staticTextMsg->Show( false ); } - for( unsigned col = 0; col < aItemHeaders.Count(); ++col ) - { - m_listBox->SetColumnWidth( col, wxLIST_AUTOSIZE ); - int columnwidth = m_listBox->GetColumnWidth( col ); - m_listBox->SetColumnWidth( col, wxLIST_AUTOSIZE_USEHEADER ); - int headerwidth = m_listBox->GetColumnWidth( col ); - m_listBox->SetColumnWidth( col, std::max( columnwidth, headerwidth ) ); - } - if( !!aSelection ) { for( unsigned row = 0; row < m_itemsListCp->size(); ++row ) @@ -177,18 +169,25 @@ void EDA_LIST_DIALOG::InsertItems( const std::vector< wxArrayString >& itemList, { wxASSERT( (int) itemList[row].GetCount() == m_listBox->GetColumnCount() ); - long itemIndex = 0; for( unsigned col = 0; col < itemList[row].GetCount(); col++ ) { + wxListItem info; + info.m_itemId = row + position; + info.m_col = col; + info.m_text = itemList[row].Item( col ); + info.m_width = DEFAULT_COL_WIDTHS[ col ]; + info.m_mask = wxLIST_MASK_TEXT | wxLIST_MASK_WIDTH; if( col == 0 ) { - itemIndex = m_listBox->InsertItem( row+position, itemList[row].Item( col ) ); - m_listBox->SetItemPtrData( itemIndex, wxUIntPtr( &itemList[row].Item( col ) ) ); + info.m_data = wxUIntPtr( &itemList[row].Item( col ) ); + info.m_mask |= wxLIST_MASK_DATA; + + m_listBox->InsertItem( info ); } else { - m_listBox->SetItem( itemIndex, col, itemList[row].Item( col ) ); + m_listBox->SetItem( info ); } } } diff --git a/common/fp_lib_table.cpp b/common/fp_lib_table.cpp index a51a1c94fc..e83c326f9d 100644 --- a/common/fp_lib_table.cpp +++ b/common/fp_lib_table.cpp @@ -235,6 +235,28 @@ void FP_LIB_TABLE::Format( OUTPUTFORMATTER* aOutput, int aIndentLevel ) const } +long long FP_LIB_TABLE::GenLastModifiedChecksum( const wxString* aNickname ) +{ + if( aNickname ) + { + const FP_LIB_TABLE_ROW* row = FindRow( *aNickname ); + wxASSERT( (PLUGIN*) row->plugin ); + return row->plugin->GetLibModificationTime( *aNickname ).GetValue().GetValue(); + } + + long long hash = 0; + + for( wxString const& nickname : GetLogicalLibs() ) + { + const FP_LIB_TABLE_ROW* row = FindRow( nickname ); + wxASSERT( (PLUGIN*) row->plugin ); + hash += row->plugin->GetLibModificationTime( nickname ).GetValue().GetValue(); + } + + return hash; +} + + void FP_LIB_TABLE::FootprintEnumerate( wxArrayString& aFootprintNames, const wxString& aNickname ) { const FP_LIB_TABLE_ROW* row = FindRow( aNickname ); diff --git a/include/fp_lib_table.h b/include/fp_lib_table.h index 64c561f20b..ffa09d3fbf 100644 --- a/include/fp_lib_table.h +++ b/include/fp_lib_table.h @@ -151,6 +151,12 @@ public: */ void FootprintEnumerate( wxArrayString& aFootprintNames, const wxString& aNickname ); + /** + * Generate a checksum of the last-mod-date of \a aNickname's directory, or a checksum + * of all the libraries' directories if \a aNickname is NULL. + */ + long long GenLastModifiedChecksum( const wxString* aNickname ); + /** * Function PrefetchLib * If possible, prefetches the specified library (e.g. performing downloads). Does not parse. diff --git a/pcbnew/footprint_info_impl.cpp b/pcbnew/footprint_info_impl.cpp index 56bf260cbb..4afb7159fd 100644 --- a/pcbnew/footprint_info_impl.cpp +++ b/pcbnew/footprint_info_impl.cpp @@ -123,11 +123,18 @@ void FOOTPRINT_LIST_IMPL::loader_job() bool FOOTPRINT_LIST_IMPL::ReadFootprintFiles( FP_LIB_TABLE* aTable, const wxString* aNickname ) { + if( aTable->GenLastModifiedChecksum( aNickname ) == m_libraries_last_mod_checksum ) + return true; + FOOTPRINT_ASYNC_LOADER loader; loader.SetList( this ); loader.Start( aTable, aNickname ); - return loader.Join(); + bool retval = loader.Join(); + + m_libraries_last_mod_checksum = aTable->GenLastModifiedChecksum( aNickname ); + + return retval; } diff --git a/pcbnew/footprint_info_impl.h b/pcbnew/footprint_info_impl.h index fd539655e9..14c80acbac 100644 --- a/pcbnew/footprint_info_impl.h +++ b/pcbnew/footprint_info_impl.h @@ -62,6 +62,7 @@ class FOOTPRINT_LIST_IMPL : public FOOTPRINT_LIST SYNC_QUEUE m_queue_out; std::atomic_bool m_first_to_finish; std::atomic_size_t m_count_finished; + long long m_libraries_last_mod_checksum; /** * Call aFunc, pushing any IO_ERRORs and std::exceptions it throws onto m_errors. diff --git a/pcbnew/io_mgr.h b/pcbnew/io_mgr.h index ae829e95ff..9435c43712 100644 --- a/pcbnew/io_mgr.h +++ b/pcbnew/io_mgr.h @@ -28,6 +28,7 @@ #include #include #include +#include class BOARD; class PLUGIN; @@ -352,6 +353,15 @@ public: virtual void FootprintEnumerate( wxArrayString& aFootprintNames, const wxString& aLibraryPath, const PROPERTIES* aProperties = NULL ); + /** + * Return the footprint lib last-mod-time, if available. + */ + virtual wxDateTime GetLibModificationTime( const wxString& aLibraryPath ) const + { + // Default implementation. + return wxDateTime::Now(); // If we don't know then we must assume the worst. + } + /** * Function PrefetchLib * If possible, prefetches the specified library (e.g. performing downloads). Does not parse. diff --git a/pcbnew/kicad_plugin.cpp b/pcbnew/kicad_plugin.cpp index 87d1428c0e..7d2d9d2aae 100644 --- a/pcbnew/kicad_plugin.cpp +++ b/pcbnew/kicad_plugin.cpp @@ -200,6 +200,7 @@ public: FP_CACHE::FP_CACHE( PCB_IO* aOwner, const wxString& aLibraryPath ) + : m_mod_time( 0.0 ) { m_owner = aOwner; m_lib_path.SetPath( aLibraryPath ); @@ -208,6 +209,9 @@ FP_CACHE::FP_CACHE( PCB_IO* aOwner, const wxString& aLibraryPath ) wxDateTime FP_CACHE::GetLibModificationTime() const { + if( !m_lib_path.DirExists() ) + return wxDateTime::Now(); + return m_lib_path.GetModificationTime(); } @@ -368,57 +372,7 @@ bool FP_CACHE::IsPath( const wxString& aPath ) const bool FP_CACHE::IsModified( const wxString& aLibPath, const wxString& aFootprintName ) const { - // The library is modified if the library path got deleted or changed. - if( !m_lib_path.DirExists() || !IsPath( aLibPath ) ) - return true; - - wxLogTrace( traceFootprintLibrary, wxT( "File '%s', m_mod_time %s-%s, file mod time: %s-%s." ), - GetChars( m_lib_path.GetPath() ), - GetChars( m_mod_time.FormatDate() ), GetChars( m_mod_time.FormatTime() ), - GetChars( m_lib_path.GetModificationTime().FormatDate() ), - GetChars( m_lib_path.GetModificationTime().FormatTime() ) ); - - // If a file is added to or remove from the library path, the cache needs reloaded. - if( m_mod_time != m_lib_path.GetModificationTime() ) - return true; - - // If no footprint was specified, check every file modification time against the time - // it was loaded. - if( aFootprintName.IsEmpty() ) - { - for( MODULE_CITER it = m_modules.begin(); it != m_modules.end(); ++it ) - { - wxFileName fn = m_lib_path; - - fn.SetName( it->second->GetFileName().GetName() ); - fn.SetExt( KiCadFootprintFileExtension ); - - if( !fn.FileExists() ) - { - wxLogTrace( traceFootprintLibrary, - wxT( "Footprint cache file '%s' does not exist." ), - fn.GetFullPath().GetData() ); - return true; - } - - if( it->second->IsModified() ) - { - wxLogTrace( traceFootprintLibrary, - wxT( "Footprint cache file '%s' has been modified." ), - fn.GetFullPath().GetData() ); - return true; - } - } - } - else - { - MODULE_CITER it = m_modules.find( aFootprintName ); - - if( it == m_modules.end() || it->second->IsModified() ) - return true; - } - - return false; + return GetLibModificationTime() > m_mod_time; } @@ -2150,6 +2104,16 @@ void PCB_IO::FootprintDelete( const wxString& aLibraryPath, const wxString& aFoo } + +wxDateTime PCB_IO::GetLibModificationTime( const wxString& aLibraryPath ) const +{ + if( !m_cache ) + return wxDateTime::Now(); // force a load + + return m_cache->GetLibModificationTime(); +} + + void PCB_IO::FootprintLibCreate( const wxString& aLibraryPath, const PROPERTIES* aProperties ) { if( wxDir::Exists( aLibraryPath ) ) diff --git a/pcbnew/kicad_plugin.h b/pcbnew/kicad_plugin.h index 0c0792a36b..c5ae9a13a0 100644 --- a/pcbnew/kicad_plugin.h +++ b/pcbnew/kicad_plugin.h @@ -43,7 +43,7 @@ class NETINFO_MAPPING; // // went to 32 Cu layers from 16. //#define SEXPR_BOARD_FILE_VERSION 20160815 // differential pair settings per net class //#define SEXPR_BOARD_FILE_VERSION 20170123 // EDA_TEXT refactor, moved 'hide' -//#define SEXPR_BOARD_FILE_VERSION 20170920 // long pad names and custom pad shape +//#define SEXPR_BOARD_FILE_VERSION 20170920 // long pad names and custom pad shape //#define SEXPR_BOARD_FILE_VERSION 20170922 // Keepout zones can exist on multiple layers //#define SEXPR_BOARD_FILE_VERSION 20171114 // Save 3D model offset in mm, instead of inches //#define SEXPR_BOARD_FILE_VERSION 20171125 // Locked/unlocked TEXTE_MODULE @@ -130,6 +130,8 @@ public: void FootprintDelete( const wxString& aLibraryPath, const wxString& aFootprintName, const PROPERTIES* aProperties = NULL ) override; + wxDateTime GetLibModificationTime( const wxString& aLibraryPath ) const override; + void FootprintLibCreate( const wxString& aLibraryPath, const PROPERTIES* aProperties = NULL) override; bool FootprintLibDelete( const wxString& aLibraryPath, const PROPERTIES* aProperties = NULL ) override;