FolderMemoryCard: Reduce unnecessary file I/O by only flushing files that have actually changed since the last known memory card state on the host file system.

This means that we are now no longer touching files that haven't technically been written to. Some games use timestamp information to automatically highlight the save that was last written to, so this should fix a small but annoying bug where it would highlight the wrong one.

Do note that while there is a much simpler check that looks like this:
	// Remove (== don't flush) all memory card pages that haven't actually changed.
	for ( auto oldIt = m_oldDataCache.begin(); oldIt != m_oldDataCache.end(); ++oldIt ) {
		auto newIt = m_cache.find( oldIt->first );
		assert( newIt != m_cache.end() ); // if this isn't true something broke somewhere, the two maps should always contain the same pages
		if ( memcmp( &oldIt->second.raw[0], &newIt->second.raw[0], PageSize ) == 0 ) {
			m_cache.erase( newIt );
		}
	}
	m_oldDataCache.clear();
It can fail in edge cases that don't actually seem too unlikely. Imagine a save being deleted, and then a new save from the same game but in a different slot being created quickly afterwards. It seems quite possible that the new save's file data then occupies the exact same pages as the old save's, and since it's from the same game it might be close enough to where a page sized section (juse 0x200 bytes!) matches the data from the old save that previously resided in that location -- which would cause this code to throw away and not flush this data! It's a shame too, since this variant would be a few ms faster as well, but I feel it's better to be safe than sorry here.
This commit is contained in:
Admiral H. Curtiss 2015-07-03 23:01:06 +02:00
parent 03a6be28c0
commit 6bd578ccbe
2 changed files with 61 additions and 7 deletions

View File

@ -38,6 +38,7 @@ void FolderMemoryCard::InitializeInternalData() {
memset( &m_backupBlock1, 0xFF, sizeof( m_backupBlock1 ) ); memset( &m_backupBlock1, 0xFF, sizeof( m_backupBlock1 ) );
memset( &m_backupBlock2, 0xFF, sizeof( m_backupBlock2 ) ); memset( &m_backupBlock2, 0xFF, sizeof( m_backupBlock2 ) );
m_cache.clear(); m_cache.clear();
m_oldDataCache.clear();
m_fileMetadataQuickAccess.clear(); m_fileMetadataQuickAccess.clear();
m_timeLastWritten = 0; m_timeLastWritten = 0;
m_isEnabled = false; m_isEnabled = false;
@ -100,6 +101,7 @@ void FolderMemoryCard::Close() {
Flush(); Flush();
m_cache.clear(); m_cache.clear();
m_oldDataCache.clear();
m_fileMetadataQuickAccess.clear(); m_fileMetadataQuickAccess.clear();
m_lastAccessedFile.Close(); m_lastAccessedFile.Close();
} }
@ -789,6 +791,7 @@ s32 FolderMemoryCard::Save( const u8 *src, u32 adr, int size ) {
cachePage = &m_cache[page]; cachePage = &m_cache[page];
const u32 adrLoad = page * PageSizeRaw; const u32 adrLoad = page * PageSizeRaw;
ReadDataWithoutCache( &cachePage->raw[0], adrLoad, PageSize ); ReadDataWithoutCache( &cachePage->raw[0], adrLoad, PageSize );
memcpy( &m_oldDataCache[page].raw[0], &cachePage->raw[0], PageSize );
} else { } else {
cachePage = &it->second; cachePage = &it->second;
} }
@ -858,7 +861,7 @@ void FolderMemoryCard::Flush() {
FlushFileEntries(); FlushFileEntries();
// Now we have the new file system, compare it to the old one and "delete" any files that were in it before but aren't anymore. // Now we have the new file system, compare it to the old one and "delete" any files that were in it before but aren't anymore.
FlushDeletedFiles( oldFileEntryTree ); FlushDeletedFilesAndRemoveUnchangedDataFromCache( oldFileEntryTree );
// and finally, flush everything that hasn't been flushed yet // and finally, flush everything that hasn't been flushed yet
for ( uint i = 0; i < pageCount; ++i ) { for ( uint i = 0; i < pageCount; ++i ) {
@ -866,6 +869,7 @@ void FolderMemoryCard::Flush() {
} }
m_lastAccessedFile.Close(); m_lastAccessedFile.Close();
m_oldDataCache.clear();
const u64 timeFlushEnd = wxGetLocalTimeMillis().GetValue(); const u64 timeFlushEnd = wxGetLocalTimeMillis().GetValue();
Console.WriteLn( L"(FolderMcd) Done! Took %u ms.", timeFlushEnd - timeFlushStart ); Console.WriteLn( L"(FolderMcd) Done! Took %u ms.", timeFlushEnd - timeFlushStart );
@ -969,14 +973,14 @@ void FolderMemoryCard::FlushFileEntries( const u32 dirCluster, const u32 remaini
} }
} }
void FolderMemoryCard::FlushDeletedFiles( const std::vector<MemoryCardFileEntryTreeNode>& oldFileEntries ) { void FolderMemoryCard::FlushDeletedFilesAndRemoveUnchangedDataFromCache( const std::vector<MemoryCardFileEntryTreeNode>& oldFileEntries ) {
const u32 newRootDirCluster = m_superBlock.data.rootdir_cluster; const u32 newRootDirCluster = m_superBlock.data.rootdir_cluster;
const u32 newFileCount = m_fileEntryDict[newRootDirCluster].entries[0].entry.data.length; const u32 newFileCount = m_fileEntryDict[newRootDirCluster].entries[0].entry.data.length;
wxString path = L""; wxString path = L"";
FlushDeletedFiles( oldFileEntries, newRootDirCluster, newFileCount, path ); FlushDeletedFilesAndRemoveUnchangedDataFromCache( oldFileEntries, newRootDirCluster, newFileCount, path );
} }
void FolderMemoryCard::FlushDeletedFiles( const std::vector<MemoryCardFileEntryTreeNode>& oldFileEntries, const u32 newCluster, const u32 newFileCount, const wxString& dirPath ) { void FolderMemoryCard::FlushDeletedFilesAndRemoveUnchangedDataFromCache( const std::vector<MemoryCardFileEntryTreeNode>& oldFileEntries, const u32 newCluster, const u32 newFileCount, const wxString& dirPath ) {
// go through all file entires of the current directory of the old data // go through all file entires of the current directory of the old data
for ( auto it = oldFileEntries.cbegin(); it != oldFileEntries.cend(); ++it ) { for ( auto it = oldFileEntries.cbegin(); it != oldFileEntries.cend(); ++it ) {
const MemoryCardFileEntry* entry = &it->entry; const MemoryCardFileEntry* entry = &it->entry;
@ -1003,12 +1007,45 @@ void FolderMemoryCard::FlushDeletedFiles( const std::vector<MemoryCardFileEntryT
FileAccessHelper::CleanMemcardFilename( cleanName ); FileAccessHelper::CleanMemcardFilename( cleanName );
const wxString subDirName = wxString::FromAscii( cleanName ); const wxString subDirName = wxString::FromAscii( cleanName );
const wxString subDirPath = dirPath + L"/" + subDirName; const wxString subDirPath = dirPath + L"/" + subDirName;
FlushDeletedFiles( it->subdir, newEntry->entry.data.cluster, newEntry->entry.data.length, subDirPath ); FlushDeletedFilesAndRemoveUnchangedDataFromCache( it->subdir, newEntry->entry.data.cluster, newEntry->entry.data.length, subDirPath );
} else if ( entry->IsFile() ) {
// still exists and is a file, see if we can remove unchanged data from m_cache
RemoveUnchangedDataFromCache( entry, newEntry );
} }
} }
} }
} }
void FolderMemoryCard::RemoveUnchangedDataFromCache( const MemoryCardFileEntry* const oldEntry, const MemoryCardFileEntry* const newEntry ) {
// Disclaimer: Technically, to actually prove that file data has not changed and still belongs to the same file, we'd need to keep a copy
// of the old FAT cluster chain and compare that as well, and only acknowledge the file as unchanged if none of those have changed. However,
// the chain of events that leads to a file having the exact same file contents as a deleted old file while also being placed in the same
// data clusters as the deleted file AND matching this condition here, in a quick enough succession that no flush has occurred yet since the
// deletion of that old file is incredibly unlikely, so I'm not sure if it's actually worth coding for.
if ( oldEntry->entry.data.timeModified != newEntry->entry.data.timeModified || oldEntry->entry.data.timeCreated != newEntry->entry.data.timeCreated
|| oldEntry->entry.data.length != newEntry->entry.data.length || oldEntry->entry.data.cluster != newEntry->entry.data.cluster ) {
return;
}
u32 cluster = newEntry->entry.data.cluster & NextDataClusterMask;
const u32 alloc_offset = m_superBlock.data.alloc_offset;
while ( cluster != LastDataCluster ) {
for ( int i = 0; i < 2; ++i ) {
const u32 page = ( cluster + alloc_offset ) * 2 + i;
auto newIt = m_cache.find( page );
if ( newIt == m_cache.end() ) { continue; }
auto oldIt = m_oldDataCache.find( page );
if ( oldIt == m_oldDataCache.end() ) { continue; }
if ( memcmp( &oldIt->second.raw[0], &newIt->second.raw[0], PageSize ) == 0 ) {
m_cache.erase( newIt );
}
}
cluster = m_fat.data[0][0][cluster] & NextDataClusterMask;
}
}
s32 FolderMemoryCard::WriteWithoutCache( const u8 *src, u32 adr, int size ) { s32 FolderMemoryCard::WriteWithoutCache( const u8 *src, u32 adr, int size ) {
const u32 block = adr / BlockSizeRaw; const u32 block = adr / BlockSizeRaw;
const u32 cluster = adr / ClusterSizeRaw; const u32 cluster = adr / ClusterSizeRaw;

View File

@ -84,6 +84,14 @@ struct MemoryCardFileEntryDateTime {
return t; return t;
} }
bool operator==( const MemoryCardFileEntryDateTime& other ) const {
return unused == other.unused && second == other.second && minute == other.minute && hour == other.hour
&& day == other.day && month == other.month && year == other.year;
}
bool operator!=( const MemoryCardFileEntryDateTime& other ) const {
return !( *this == other );
}
}; };
#pragma pack(pop) #pragma pack(pop)
@ -254,6 +262,10 @@ protected:
// holds a copy of modified pages of the memory card before they're flushed to the file system // holds a copy of modified pages of the memory card before they're flushed to the file system
std::map<u32, MemoryCardPage> m_cache; std::map<u32, MemoryCardPage> m_cache;
// contains the state of how the data looked before the first write to it
// used to reduce the amount of disk I/O by not re-writing unchanged data that just happened to be
// touched in memory due to how actual physical memory cards have to erase and rewrite in blocks
std::map<u32, MemoryCardPage> m_oldDataCache;
// if > 0, the amount of frames until data is flushed to the file system // if > 0, the amount of frames until data is flushed to the file system
// reset to FramesAfterWriteUntilFlush on each write // reset to FramesAfterWriteUntilFlush on each write
int m_framesUntilFlush; int m_framesUntilFlush;
@ -434,13 +446,18 @@ protected:
void FlushFileEntries( const u32 dirCluster, const u32 remainingFiles, const wxString& dirPath = L"", MemoryCardFileMetadataReference* parent = nullptr ); void FlushFileEntries( const u32 dirCluster, const u32 remainingFiles, const wxString& dirPath = L"", MemoryCardFileMetadataReference* parent = nullptr );
// "delete" (prepend '_pcsx2_deleted_' to) any files that exist in oldFileEntries but no longer exist in m_fileEntryDict // "delete" (prepend '_pcsx2_deleted_' to) any files that exist in oldFileEntries but no longer exist in m_fileEntryDict
void FlushDeletedFiles( const std::vector<MemoryCardFileEntryTreeNode>& oldFileEntries ); // also calls RemoveUnchangedDataFromCache() since both operate on comparing with the old file entires
void FlushDeletedFilesAndRemoveUnchangedDataFromCache( const std::vector<MemoryCardFileEntryTreeNode>& oldFileEntries );
// recursive worker method of the above // recursive worker method of the above
// - newCluster: Current directory dotdir cluster of the new entries. // - newCluster: Current directory dotdir cluster of the new entries.
// - newFileCount: Number of file entries in the new directory. // - newFileCount: Number of file entries in the new directory.
// - dirPath: Path to the current directory relative to the root of the memcard. Must be identical for both entries. // - dirPath: Path to the current directory relative to the root of the memcard. Must be identical for both entries.
void FlushDeletedFiles( const std::vector<MemoryCardFileEntryTreeNode>& oldFileEntries, const u32 newCluster, const u32 newFileCount, const wxString& dirPath ); void FlushDeletedFilesAndRemoveUnchangedDataFromCache( const std::vector<MemoryCardFileEntryTreeNode>& oldFileEntries, const u32 newCluster, const u32 newFileCount, const wxString& dirPath );
// try and remove unchanged data from m_cache
// oldEntry and newEntry should be equivalent entries found by FindEquivalent()
void RemoveUnchangedDataFromCache( const MemoryCardFileEntry* const oldEntry, const MemoryCardFileEntry* const newEntry );
// write data as Save() normally would, but ignore the cache; used for flushing // write data as Save() normally would, but ignore the cache; used for flushing
s32 WriteWithoutCache( const u8 *src, u32 adr, int size ); s32 WriteWithoutCache( const u8 *src, u32 adr, int size );