Go back to checking individual file timestamps.

Too many external applications fail to touch the parent directory.

Also removes FP_CACHE_ITEM lastMod times and dirty flags as we've
always loaded libraries atomically anyway.

Claws back some of the performance lost by being more efficient
with cache management for sequential calls to Enumerate and then
Load.

Fixes: lp:1750936
* https://bugs.launchpad.net/kicad/+bug/1750936
This commit is contained in:
Jeff Young 2018-02-23 12:21:06 +00:00 committed by Wayne Stambaugh
parent a83669ab19
commit 7bd2f14342
9 changed files with 176 additions and 126 deletions

View File

@ -235,13 +235,13 @@ void FP_LIB_TABLE::Format( OUTPUTFORMATTER* aOutput, int aIndentLevel ) const
}
long long FP_LIB_TABLE::GenLastModifiedChecksum( const wxString* aNickname )
long long FP_LIB_TABLE::GenerateTimestamp( const wxString* aNickname )
{
if( aNickname )
{
const FP_LIB_TABLE_ROW* row = FindRow( *aNickname );
wxASSERT( (PLUGIN*) row->plugin );
return row->plugin->GetLibModificationTime( *aNickname ).GetValue().GetValue();
return row->plugin->GetLibraryTimestamp();
}
long long hash = 0;
@ -250,7 +250,7 @@ long long FP_LIB_TABLE::GenLastModifiedChecksum( const wxString* aNickname )
{
const FP_LIB_TABLE_ROW* row = FindRow( nickname );
wxASSERT( (PLUGIN*) row->plugin );
hash += row->plugin->GetLibModificationTime( nickname ).GetValue().GetValue();
hash += row->plugin->GetLibraryTimestamp();
}
return hash;
@ -297,6 +297,45 @@ const FP_LIB_TABLE_ROW* FP_LIB_TABLE::FindRow( const wxString& aNickname )
}
static void setLibNickname( MODULE* aModule,
const wxString& aNickname, const wxString& aFootprintName )
{
// The library cannot know its own name, because it might have been renamed or moved.
// Therefore footprints cannot know their own library nickname when residing in
// a footprint library.
// Only at this API layer can we tell the footprint about its actual library nickname.
if( aModule )
{
// remove "const"-ness, I really do want to set nickname without
// having to copy the LIB_ID and its two strings, twice each.
LIB_ID& fpid = (LIB_ID&) aModule->GetFPID();
// Catch any misbehaving plugin, which should be setting internal footprint name properly:
wxASSERT( aFootprintName == fpid.GetLibItemName().wx_str() );
// and clearing nickname
wxASSERT( !fpid.GetLibNickname().size() );
fpid.SetLibNickname( aNickname );
}
}
MODULE* FP_LIB_TABLE::LoadEnumeratedFootprint( const wxString& aNickname,
const wxString& aFootprintName )
{
const FP_LIB_TABLE_ROW* row = FindRow( aNickname );
wxASSERT( (PLUGIN*) row->plugin );
MODULE* ret = row->plugin->LoadEnumeratedFootprint( row->GetFullURI( true ), aFootprintName,
row->GetProperties() );
setLibNickname( ret, row->GetNickName(), aFootprintName );
return ret;
}
MODULE* FP_LIB_TABLE::FootprintLoad( const wxString& aNickname, const wxString& aFootprintName )
{
const FP_LIB_TABLE_ROW* row = FindRow( aNickname );
@ -305,24 +344,7 @@ MODULE* FP_LIB_TABLE::FootprintLoad( const wxString& aNickname, const wxString&
MODULE* ret = row->plugin->FootprintLoad( row->GetFullURI( true ), aFootprintName,
row->GetProperties() );
// The library cannot know its own name, because it might have been renamed or moved.
// Therefore footprints cannot know their own library nickname when residing in
// a footprint library.
// Only at this API layer can we tell the footprint about its actual library nickname.
if( ret )
{
// remove "const"-ness, I really do want to set nickname without
// having to copy the LIB_ID and its two strings, twice each.
LIB_ID& fpid = (LIB_ID&) ret->GetFPID();
// Catch any misbehaving plugin, which should be setting internal footprint name properly:
wxASSERT( aFootprintName == fpid.GetLibItemName().wx_str() );
// and clearing nickname
wxASSERT( !fpid.GetLibNickname().size() );
fpid.SetLibNickname( row->GetNickName() );
}
setLibNickname( ret, row->GetNickName(), aFootprintName );
return ret;
}

View File

@ -152,10 +152,10 @@ 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.
* Generate a hashed timestamp representing the last-mod-times of the library indicated
* by \a aNickname, or all libraries if \a aNickname is NULL.
*/
long long GenLastModifiedChecksum( const wxString* aNickname );
long long GenerateTimestamp( const wxString* aNickname );
/**
* Function PrefetchLib
@ -186,6 +186,13 @@ public:
*/
MODULE* FootprintLoad( const wxString& aNickname, const wxString& aFootprintName );
/**
* Function LoadEnumeratedFootprint
*
* a version of FootprintLoad() for use after FootprintEnumerate() for more efficient
* cache management.
*/
MODULE* LoadEnumeratedFootprint( const wxString& aNickname, const wxString& aFootprintName );
/**
* Enum SAVE_T
* is the set of return values from FootprintSave() below.

View File

@ -47,8 +47,7 @@ void FOOTPRINT_INFO_IMPL::load()
wxASSERT( fptable );
std::unique_ptr<MODULE> footprint( fptable->FootprintLoad( m_nickname, m_fpname ) );
std::unique_ptr<MODULE> footprint( fptable->LoadEnumeratedFootprint( m_nickname, m_fpname ) );
if( footprint.get() == NULL ) // Should happen only with malformed/broken libraries
{
m_pad_count = 0;
@ -119,7 +118,7 @@ void FOOTPRINT_LIST_IMPL::loader_job()
bool FOOTPRINT_LIST_IMPL::ReadFootprintFiles( FP_LIB_TABLE* aTable, const wxString* aNickname,
WX_PROGRESS_REPORTER* aProgressReporter )
{
if( aTable->GenLastModifiedChecksum( aNickname ) == m_libraries_last_mod_checksum )
if( aTable->GenerateTimestamp( aNickname ) == m_list_timestamp )
return true;
m_progress_reporter = aProgressReporter;
@ -167,7 +166,7 @@ bool FOOTPRINT_LIST_IMPL::ReadFootprintFiles( FP_LIB_TABLE* aTable, const wxStri
( _( "Loading incomplete; cancelled by user." ), nullptr, nullptr, 0 ) );
}
m_libraries_last_mod_checksum = aTable->GenLastModifiedChecksum( aNickname );
m_list_timestamp = aTable->GenerateTimestamp( aNickname );
m_progress_reporter = nullptr;
return m_errors.empty();
@ -238,7 +237,7 @@ bool FOOTPRINT_LIST_IMPL::JoinWorkers()
try
{
this->m_lib_table->FootprintEnumerate( fpnames, nickname );
m_lib_table->FootprintEnumerate( fpnames, nickname );
}
catch( const IO_ERROR& ioe )
{
@ -306,7 +305,7 @@ size_t FOOTPRINT_LIST_IMPL::CountFinished()
FOOTPRINT_LIST_IMPL::FOOTPRINT_LIST_IMPL() :
m_loader( nullptr ),
m_count_finished( 0 ),
m_libraries_last_mod_checksum( 0 ),
m_list_timestamp( 0 ),
m_progress_reporter( nullptr ),
m_cancelled( false )
{

View File

@ -62,7 +62,7 @@ class FOOTPRINT_LIST_IMPL : public FOOTPRINT_LIST
SYNC_QUEUE<wxString> m_queue_in;
SYNC_QUEUE<wxString> m_queue_out;
std::atomic_size_t m_count_finished;
long long m_libraries_last_mod_checksum;
long long m_list_timestamp;
WX_PROGRESS_REPORTER* m_progress_reporter;
std::atomic_bool m_cancelled;

View File

@ -354,12 +354,14 @@ public:
const PROPERTIES* aProperties = NULL );
/**
* Return the footprint lib last-mod-time, if available.
* Generate a timestamp representing all the files in the library (including the library
* directory).
* Timestamps should not be considered ordered; they either match or they don't.
*/
virtual wxDateTime GetLibModificationTime( const wxString& aLibraryPath ) const
virtual long long GetLibraryTimestamp() const
{
// Default implementation.
return wxDateTime::Now(); // If we don't know then we must assume the worst.
return 0;
}
/**
@ -408,6 +410,15 @@ public:
virtual MODULE* FootprintLoad( const wxString& aLibraryPath, const wxString& aFootprintName,
const PROPERTIES* aProperties = NULL );
/**
* Function LoadEnumeratedFootprint
* a version of FootprintLoad() for use after FootprintEnumerate() for more
* efficient cache management.
*/
virtual MODULE* LoadEnumeratedFootprint( const wxString& aLibraryPath,
const wxString& aFootprintName,
const PROPERTIES* aProperties = NULL );
/**
* Function FootprintSave
* will write @a aModule to an existing library located at @a aLibraryPath.

View File

@ -92,7 +92,6 @@ void filterNetClass( const BOARD& aBoard, NETCLASS& aNetClass )
class FP_CACHE_ITEM
{
wxFileName m_file_name; ///< The the full file name and path of the footprint to cache.
wxDateTime m_mod_time; ///< The last file modified time stamp.
std::unique_ptr<MODULE> m_module;
public:
@ -101,11 +100,7 @@ public:
wxString GetName() const { return m_file_name.GetDirs().Last(); }
wxFileName GetFileName() const { return m_file_name; }
/// Tell if the disk content or the lib_path has changed.
bool IsModified() const;
MODULE* GetModule() const { return m_module.get(); }
void UpdateModificationTime() { m_mod_time = m_file_name.GetModificationTime(); }
};
@ -113,26 +108,6 @@ FP_CACHE_ITEM::FP_CACHE_ITEM( MODULE* aModule, const wxFileName& aFileName ) :
m_module( aModule )
{
m_file_name = aFileName;
if( m_file_name.FileExists() )
m_mod_time = m_file_name.GetModificationTime();
else
m_mod_time.Now();
}
bool FP_CACHE_ITEM::IsModified() const
{
if( !m_file_name.FileExists() )
return false;
wxLogTrace( traceFootprintLibrary, wxT( "File '%s', m_mod_time %s-%s, file mod time: %s-%s." ),
GetChars( m_file_name.GetFullPath() ),
GetChars( m_mod_time.FormatDate() ), GetChars( m_mod_time.FormatTime() ),
GetChars( m_file_name.GetModificationTime().FormatDate() ),
GetChars( m_file_name.GetModificationTime().FormatTime() ) );
return m_file_name.GetModificationTime() != m_mod_time;
}
@ -145,20 +120,16 @@ class FP_CACHE
{
PCB_IO* m_owner; /// Plugin object that owns the cache.
wxFileName m_lib_path; /// The path of the library.
wxDateTime m_mod_time; /// Footprint library path modified time stamp.
MODULE_MAP m_modules; /// Map of footprint file name per MODULE*.
bool m_represents_non_existant_dir; // Indicates this cache represents a directory
// which does not exist. This forces
// GetLibModificationTime() to return 0 (at least
// until the directory is found, at which point it
// will return Now).
bool m_cache_dirty; // Stored separately because it's expensive
// to check m_cache_timestamp against file.
long long m_cache_timestamp; // A timestamp for the footprint file.
public:
FP_CACHE( PCB_IO* aOwner, const wxString& aLibraryPath );
wxString GetPath() const { return m_lib_path.GetPath(); }
wxDateTime GetLastModificationTime() const { return m_mod_time; }
bool IsWritable() const { return m_lib_path.IsOk() && m_lib_path.IsDirWritable(); }
MODULE_MAP& GetModules() { return m_modules; }
@ -173,21 +144,19 @@ public:
void Remove( const wxString& aFootprintName );
wxDateTime GetLibModificationTime() const;
/**
* Function GetTimestamp
* Generate a timestamp representing all source files in the cache (including the
* parent directory).
* Timestamps should not be considered ordered. They either match or they don't.
*/
long long GetTimestamp();
/**
* Function IsModified
* check if the footprint cache has been modified relative to \a aLibPath
* and \a aFootprintName.
*
* @param aLibPath is a path to test the current cache library path against.
* @param aFootprintName is the footprint name in the cache to test. If the footprint
* name is empty, the all the footprint files in the library are
* checked to see if they have been modified.
* @return true if the cache has been modified.
* Return true if the cache is not up-to-date.
*/
bool IsModified( const wxString& aLibPath,
const wxString& aFootprintName = wxEmptyString ) const;
bool IsModified();
/**
* Function IsPath
@ -206,30 +175,18 @@ public:
FP_CACHE::FP_CACHE( PCB_IO* aOwner, const wxString& aLibraryPath )
: m_mod_time( 0.0 )
{
m_owner = aOwner;
m_lib_path.SetPath( aLibraryPath );
m_represents_non_existant_dir = false;
}
wxDateTime FP_CACHE::GetLibModificationTime() const
{
if( !m_lib_path.DirExists() )
{
if( m_represents_non_existant_dir )
return wxDateTime( 0.0 );
else
return wxDateTime::Now();
}
return m_lib_path.GetModificationTime();
m_cache_timestamp = 0;
m_cache_dirty = true;
}
void FP_CACHE::Save()
{
m_cache_timestamp = 0;
if( !m_lib_path.DirExists() && !m_lib_path.Mkdir() )
{
THROW_IO_ERROR( wxString::Format( _( "Cannot create footprint library path \"%s\"" ),
@ -246,9 +203,6 @@ void FP_CACHE::Save()
{
wxFileName fn = it->second->GetFileName();
if( fn.FileExists() && !it->second->IsModified() )
continue;
wxString tempFileName =
#ifdef USE_TMP_FILE
fn.CreateTempFileName( fn.GetPath() );
@ -284,21 +238,22 @@ void FP_CACHE::Save()
THROW_IO_ERROR( msg );
}
#endif
it->second->UpdateModificationTime();
m_mod_time = GetLibModificationTime();
m_cache_timestamp += fn.GetModificationTime().GetValue().GetValue();
}
m_cache_timestamp += m_lib_path.GetModificationTime().GetValue().GetValue();
m_cache_dirty = false;
}
void FP_CACHE::Load()
{
m_represents_non_existant_dir = false;
wxDir dir( m_lib_path.GetPath() );
if( !dir.IsOpened() )
{
m_represents_non_existant_dir = true;
m_cache_timestamp = 0;
m_cache_dirty = false;
wxString msg = wxString::Format(
_( "Footprint library path \"%s\" does not exist" ),
@ -307,6 +262,11 @@ void FP_CACHE::Load()
THROW_IO_ERROR( msg );
}
else
{
m_cache_timestamp = m_lib_path.GetModificationTime().GetValue().GetValue();
m_cache_dirty = false;
}
wxString fpFileName;
wxString wildcard = wxT( "*." ) + KiCadFootprintFileExtension;
@ -334,6 +294,8 @@ void FP_CACHE::Load()
footprint->SetFPID( LIB_ID( fpName ) );
m_modules.insert( fpName, new FP_CACHE_ITEM( footprint, fullPath ) );
m_cache_timestamp += fullPath.GetModificationTime().GetValue().GetValue();
}
catch( const IO_ERROR& ioe )
{
@ -344,11 +306,6 @@ void FP_CACHE::Load()
}
} while( dir.GetNext( &fpFileName ) );
// Remember the file modification time of library file when the
// cache snapshot was made, so that in a networked environment we will
// reload the cache as needed.
m_mod_time = GetLibModificationTime();
if( !cacheError.IsEmpty() )
THROW_IO_ERROR( cacheError );
}
@ -386,9 +343,41 @@ bool FP_CACHE::IsPath( const wxString& aPath ) const
}
bool FP_CACHE::IsModified( const wxString& aLibPath, const wxString& aFootprintName ) const
bool FP_CACHE::IsModified()
{
return GetLibModificationTime() > m_mod_time;
if( m_cache_dirty )
return true;
else
return GetTimestamp() != m_cache_timestamp;
}
long long FP_CACHE::GetTimestamp()
{
// Avoid expensive GetModificationTime checks if we already know we're dirty
if( m_cache_dirty )
return wxDateTime::Now().GetValue().GetValue();
long long files_timestamp = 0;
if( m_lib_path.DirExists() )
{
files_timestamp = m_lib_path.GetModificationTime().GetValue().GetValue();
for( MODULE_CITER it = m_modules.begin(); it != m_modules.end(); ++it )
{
wxFileName moduleFile = it->second->GetFileName();
if( moduleFile.FileExists() )
files_timestamp += moduleFile.GetModificationTime().GetValue().GetValue();
}
}
// If the new timestamp doesn't match the cache timestamp, then save ourselves the
// expensive calls next time
if( m_cache_timestamp != files_timestamp )
m_cache_dirty = true;
return files_timestamp;
}
@ -1955,9 +1944,9 @@ void PCB_IO::init( const PROPERTIES* aProperties )
}
void PCB_IO::cacheLib( const wxString& aLibraryPath, const wxString& aFootprintName )
void PCB_IO::validateCache( const wxString& aLibraryPath )
{
if( !m_cache || m_cache->IsModified( aLibraryPath, aFootprintName ) )
if( !m_cache || m_cache->IsModified() )
{
// a spectacular episode in memory management:
delete m_cache;
@ -1980,7 +1969,7 @@ void PCB_IO::FootprintEnumerate( wxArrayString& aFootprintNames,
try
{
cacheLib( aLibraryPath );
validateCache( aLibraryPath );
}
catch( const IO_ERROR& ioe )
{
@ -2002,15 +1991,14 @@ void PCB_IO::FootprintEnumerate( wxArrayString& aFootprintNames,
}
MODULE* PCB_IO::FootprintLoad( const wxString& aLibraryPath, const wxString& aFootprintName,
MODULE* PCB_IO::LoadEnumeratedFootprint( const wxString& aLibraryPath,
const wxString& aFootprintName,
const PROPERTIES* aProperties )
{
LOCALE_IO toggle; // toggles on, then off, the C locale.
init( aProperties );
cacheLib( aLibraryPath, aFootprintName );
const MODULE_MAP& mods = m_cache->GetModules();
MODULE_CITER it = mods.find( aFootprintName );
@ -2025,6 +2013,17 @@ MODULE* PCB_IO::FootprintLoad( const wxString& aLibraryPath, const wxString& aFo
}
MODULE* PCB_IO::FootprintLoad( const wxString& aLibraryPath, const wxString& aFootprintName,
const PROPERTIES* aProperties )
{
LOCALE_IO toggle; // toggles on, then off, the C locale.
validateCache( aLibraryPath );
return LoadEnumeratedFootprint( aLibraryPath, aFootprintName, aProperties );
}
void PCB_IO::FootprintSave( const wxString& aLibraryPath, const MODULE* aFootprint,
const PROPERTIES* aProperties )
{
@ -2036,7 +2035,7 @@ void PCB_IO::FootprintSave( const wxString& aLibraryPath, const MODULE* aFootpri
// called for saving into a library path.
m_ctl = CTL_FOR_LIBRARY;
cacheLib( aLibraryPath );
validateCache( aLibraryPath );
if( !m_cache->IsWritable() )
{
@ -2103,7 +2102,7 @@ void PCB_IO::FootprintDelete( const wxString& aLibraryPath, const wxString& aFoo
init( aProperties );
cacheLib( aLibraryPath );
validateCache( aLibraryPath );
if( !m_cache->IsWritable() )
{
@ -2116,12 +2115,13 @@ void PCB_IO::FootprintDelete( const wxString& aLibraryPath, const wxString& aFoo
wxDateTime PCB_IO::GetLibModificationTime( const wxString& aLibraryPath ) const
long long PCB_IO::GetLibraryTimestamp() const
{
// If we have not cache, return a number which won't match any stored timestamps
if( !m_cache )
return wxDateTime::Now(); // force a load
return wxDateTime::Now().GetValue().GetValue();
return m_cache->GetLibModificationTime();
return m_cache->GetTimestamp();
}
@ -2226,7 +2226,7 @@ bool PCB_IO::IsFootprintLibWritable( const wxString& aLibraryPath )
init( NULL );
cacheLib( aLibraryPath );
validateCache( aLibraryPath );
return m_cache->IsWritable();
}

View File

@ -121,6 +121,9 @@ public:
void FootprintEnumerate( wxArrayString& aFootprintNames, const wxString& aLibraryPath,
const PROPERTIES* aProperties = NULL ) override;
MODULE* LoadEnumeratedFootprint( const wxString& aLibraryPath, const wxString& aFootprintName,
const PROPERTIES* aProperties = NULL ) override;
MODULE* FootprintLoad( const wxString& aLibraryPath, const wxString& aFootprintName,
const PROPERTIES* aProperties = NULL ) override;
@ -130,7 +133,7 @@ public:
void FootprintDelete( const wxString& aLibraryPath, const wxString& aFootprintName,
const PROPERTIES* aProperties = NULL ) override;
wxDateTime GetLibModificationTime( const wxString& aLibraryPath ) const override;
long long GetLibraryTimestamp() const override;
void FootprintLibCreate( const wxString& aLibraryPath, const PROPERTIES* aProperties = NULL) override;
@ -188,8 +191,7 @@ protected:
NETINFO_MAPPING* m_mapping; ///< mapping for net codes, so only not empty net codes
///< are stored with consecutive integers as net codes
/// we only cache one footprint library, this determines which one.
void cacheLib( const wxString& aLibraryPath, const wxString& aFootprintName = wxEmptyString );
void validateCache( const wxString& aLibraryPath );
void init( const PROPERTIES* aProperties );

View File

@ -574,7 +574,7 @@ void FOOTPRINT_EDIT_FRAME::OnSaveLibraryAs( wxCommandEvent& aEvent )
for( unsigned i = 0; i < mods.size(); ++i )
{
std::unique_ptr<MODULE> m( cur->FootprintLoad( curLibPath, mods[i] ) );
std::unique_ptr<MODULE> m( cur->LoadEnumeratedFootprint( curLibPath, mods[i] ) );
dst->FootprintSave( dstLibPath, m.get() );
msg = wxString::Format( _( "Footprint \"%s\" saved" ),

View File

@ -73,6 +73,15 @@ void PLUGIN::PrefetchLib( const wxString& aLibraryPath, const PROPERTIES* aPrope
}
MODULE* PLUGIN::LoadEnumeratedFootprint( const wxString& aLibraryPath,
const wxString& aFootprintName,
const PROPERTIES* aProperties )
{
// default implementation
return FootprintLoad( aLibraryPath, aFootprintName, aProperties );
}
MODULE* PLUGIN::FootprintLoad( const wxString& aLibraryPath, const wxString& aFootprintName,
const PROPERTIES* aProperties )
{