From 57ba38560cc9d3f60a58a58dbfa00379621bb3e6 Mon Sep 17 00:00:00 2001 From: Seth Hillbrand Date: Fri, 18 Aug 2023 13:42:39 -0700 Subject: [PATCH] Optimize library load time for Altium plugin Load times were >20min for moderate sized libraries as EnumFiles iterated over the entire list for each operation. The update modifies our third-party lib to allow a return value, stopping the iteration when we find our desired entry. This also provides a short-circuit for ASCII-based names, allowing single-level parsing if available --- common/plugins/altium/altium_parser.cpp | 156 ++++++++++++++---- common/plugins/altium/altium_parser.h | 4 +- .../plugins/altium/altium_designer_plugin.cpp | 2 + pcbnew/plugins/altium/altium_pcb.cpp | 14 +- .../compoundfilereader/compoundfilereader.h | 25 +-- 5 files changed, 153 insertions(+), 48 deletions(-) diff --git a/common/plugins/altium/altium_parser.cpp b/common/plugins/altium/altium_parser.cpp index 14a9975472..f97a2d694e 100644 --- a/common/plugins/altium/altium_parser.cpp +++ b/common/plugins/altium/altium_parser.cpp @@ -103,7 +103,7 @@ std::map ALTIUM_COMPOUND_FILE::ListLibFootprints() const m_reader->EnumFiles( root, 2, [&]( const CFB::COMPOUND_FILE_ENTRY* entry, const CFB::utf16string& dir, - int level ) -> void + int level ) -> int { std::wstring dirName = UTF16ToWstring( dir.data(), dir.size() ); std::wstring fileName = UTF16ToWstring( entry->name, entry->nameLen ); @@ -122,48 +122,126 @@ std::map ALTIUM_COMPOUND_FILE::ListLibFootprints() const patternMap.emplace( key, fpName ); } + + return 0; } ); return patternMap; } -wxString ALTIUM_COMPOUND_FILE::FindLibFootprintDirName( const wxString& aFpUnicodeName ) const +std::tuple +ALTIUM_COMPOUND_FILE::FindLibFootprintDirName( const wxString& aFpUnicodeName ) const { if( !m_reader ) - return wxEmptyString; + return std::tuple( wxEmptyString, nullptr ); const CFB::COMPOUND_FILE_ENTRY* root = m_reader->GetRootEntry(); if( !root ) - return wxEmptyString; + return std::tuple( wxEmptyString, nullptr ); - wxString ret; + wxString retStr; + const CFB::COMPOUND_FILE_ENTRY* retEntry = nullptr; - m_reader->EnumFiles( root, 2, - [&]( const CFB::COMPOUND_FILE_ENTRY* entry, const CFB::utf16string& dir, - int level ) -> void - { - std::wstring dirName = UTF16ToWstring( dir.data(), dir.size() ); - std::wstring fileName = UTF16ToWstring( entry->name, entry->nameLen ); + // Cheap and easy check first as most ASCII-coded libs are under the same dir name + m_reader->EnumFiles( root, 1, + [&]( const CFB::COMPOUND_FILE_ENTRY* tentry, const CFB::utf16string&, int ) -> int + { + // We are only looking for one string, so if we found it, break the loop + if( retStr != wxEmptyString ) + return 1; - if( m_reader->IsStream( entry ) && fileName == L"Parameters" ) - { - ALTIUM_PARSER parametersReader( *this, entry ); - std::map parameterProperties = - parametersReader.ReadProperties(); + std::wstring dirName = UTF16ToWstring( tentry->name, tentry->nameLen ); - wxString fpName = ALTIUM_PARSER::ReadUnicodeString( - parameterProperties, wxT( "PATTERN" ), wxT( "" ) ); + if( aFpUnicodeName.ToStdWstring().compare( 0, 10, dirName, 0, 10 ) ) + return 0; - if( fpName == aFpUnicodeName ) - { - ret = dirName; - } - } + m_reader->EnumFiles( tentry, 1, + [&]( const CFB::COMPOUND_FILE_ENTRY* entry, const CFB::utf16string&, int ) -> int + { + std::wstring fileName = UTF16ToWstring( entry->name, entry->nameLen ); + + if( m_reader->IsStream( entry ) && fileName == L"Parameters" ) + { + ALTIUM_PARSER parametersReader( *this, entry ); + std::map parameterProperties = + parametersReader.ReadProperties(); + + wxString fpName = ALTIUM_PARSER::ReadUnicodeString( + parameterProperties, wxT( "PATTERN" ), wxT( "" ) ); + + if( fpName == aFpUnicodeName ) + { + retStr = dirName; + return 1; + } + } + + return 0; + + } ); + + if( retStr != wxEmptyString ) + { + retEntry = tentry; + return 1; + } + + return 0; } ); - return ret; + if( retStr != wxEmptyString ) + return std::make_tuple( retStr, retEntry ); + + // Now do the expensive check, iterating through each directory in the library and reading the files + m_reader->EnumFiles( root, 1, + [&]( const CFB::COMPOUND_FILE_ENTRY* tentry, const CFB::utf16string& dir, + int level ) -> int + { + if( retStr != wxEmptyString ) + return 1; + + std::wstring dirName = UTF16ToWstring( tentry->name, tentry->nameLen ); + + m_reader->EnumFiles( tentry, 1, + [&]( const CFB::COMPOUND_FILE_ENTRY* entry, const CFB::utf16string&, int ) -> int + { + std::wstring fileName = UTF16ToWstring( entry->name, entry->nameLen ); + + if( m_reader->IsStream( entry ) && fileName == L"Parameters" ) + { + ALTIUM_PARSER parametersReader( *this, entry ); + std::map parameterProperties = + parametersReader.ReadProperties(); + + wxString fpName = ALTIUM_PARSER::ReadUnicodeString( + parameterProperties, wxT( "PATTERN" ), wxT( "" ) ); + + if( fpName == aFpUnicodeName ) + { + retStr = dirName; + return 1; + } + } + + return 0; + + } ); + + if( retStr != wxEmptyString ) + { + retEntry = tentry; + return 1; + } + + return 0; + } ); + + if( retStr != wxEmptyString ) + return std::make_tuple( retStr, retEntry ); + + return std::tuple( wxEmptyString, nullptr ); } @@ -178,41 +256,52 @@ ALTIUM_COMPOUND_FILE::FindStreamSingleLevel( const CFB::COMPOUND_FILE_ENTRY* aEn m_reader->EnumFiles( aEntry, 1, [&]( const CFB::COMPOUND_FILE_ENTRY* entry, const CFB::utf16string& dir, - int level ) -> void + int level ) -> int { + if( ret != nullptr ) + return 1; + if( m_reader->IsStream( entry ) == aIsStream ) { std::string name = UTF16ToUTF8( entry->name ); if( name == aName.c_str() ) { ret = entry; + return 1; } } + + return 0; } ); return ret; } const CFB::COMPOUND_FILE_ENTRY* -ALTIUM_COMPOUND_FILE::FindStream( const std::vector& aStreamPath ) const +ALTIUM_COMPOUND_FILE::FindStream( const CFB::COMPOUND_FILE_ENTRY* aStart, + const std::vector& aStreamPath ) const { if( !m_reader ) return nullptr; - const CFB::COMPOUND_FILE_ENTRY* currentDirEntry = m_reader->GetRootEntry(); + if( !aStart ) + aStart = m_reader->GetRootEntry(); auto it = aStreamPath.cbegin(); - while( currentDirEntry != nullptr ) + + while( aStart != nullptr ) { const std::string& name = *it; if( ++it == aStreamPath.cend() ) { - return FindStreamSingleLevel( currentDirEntry, name, true ); + const CFB::COMPOUND_FILE_ENTRY* ret = FindStreamSingleLevel( aStart, name, true ); + return ret; } else { - currentDirEntry = FindStreamSingleLevel( currentDirEntry, name, false ); + const CFB::COMPOUND_FILE_ENTRY* ret = FindStreamSingleLevel( aStart, name, false ); + aStart = ret; } } @@ -220,6 +309,13 @@ ALTIUM_COMPOUND_FILE::FindStream( const std::vector& aStreamPath ) } +const CFB::COMPOUND_FILE_ENTRY* +ALTIUM_COMPOUND_FILE::FindStream( const std::vector& aStreamPath ) const +{ + return FindStream( nullptr, aStreamPath ); +} + + ALTIUM_PARSER::ALTIUM_PARSER( const ALTIUM_COMPOUND_FILE& aFile, const CFB::COMPOUND_FILE_ENTRY* aEntry ) { diff --git a/common/plugins/altium/altium_parser.h b/common/plugins/altium/altium_parser.h index cb2b6f0002..05520f6014 100644 --- a/common/plugins/altium/altium_parser.h +++ b/common/plugins/altium/altium_parser.h @@ -65,10 +65,12 @@ public: std::map ListLibFootprints() const; - wxString FindLibFootprintDirName( const wxString& aFpUnicodeName ) const; + std::tuple FindLibFootprintDirName( const wxString& aFpUnicodeName ) const; const CFB::COMPOUND_FILE_ENTRY* FindStream( const std::vector& aStreamPath ) const; + const CFB::COMPOUND_FILE_ENTRY* FindStream( const CFB::COMPOUND_FILE_ENTRY* aStart, const std::vector& aStreamPath ) const; + const CFB::COMPOUND_FILE_ENTRY* FindStreamSingleLevel( const CFB::COMPOUND_FILE_ENTRY* aEntry, const std::string aName, const bool aIsStream ) const; diff --git a/pcbnew/plugins/altium/altium_designer_plugin.cpp b/pcbnew/plugins/altium/altium_designer_plugin.cpp index 2eed2c621c..fc2cb1d50b 100644 --- a/pcbnew/plugins/altium/altium_designer_plugin.cpp +++ b/pcbnew/plugins/altium/altium_designer_plugin.cpp @@ -162,6 +162,7 @@ void ALTIUM_DESIGNER_PLUGIN::FootprintEnumerate( wxArrayString& aFootprintNames const std::vector streamName = { "Library", "Data" }; const CFB::COMPOUND_FILE_ENTRY* libraryData = altiumLibFile.FindStream( streamName ); + if( libraryData == nullptr ) { THROW_IO_ERROR( @@ -174,6 +175,7 @@ void ALTIUM_DESIGNER_PLUGIN::FootprintEnumerate( wxArrayString& aFootprintNames uint32_t numberOfFootprints = parser.Read(); aFootprintNames.Alloc( numberOfFootprints ); + for( size_t i = 0; i < numberOfFootprints; i++ ) { parser.ReadAndSetSubrecordLength(); diff --git a/pcbnew/plugins/altium/altium_pcb.cpp b/pcbnew/plugins/altium/altium_pcb.cpp index 9ed90ab91a..198525c225 100644 --- a/pcbnew/plugins/altium/altium_pcb.cpp +++ b/pcbnew/plugins/altium/altium_pcb.cpp @@ -34,6 +34,7 @@ #include #include #include +#include #include #include @@ -606,7 +607,10 @@ FOOTPRINT* ALTIUM_PCB::ParseFootprint( const ALTIUM_COMPOUND_FILE& altiumLibFile // ParseWideStrings6Data( altiumLibFile, unicodeStringsData ); // } - wxString fpDirName = altiumLibFile.FindLibFootprintDirName(aFootprintName); + std::tuple ret = altiumLibFile.FindLibFootprintDirName(aFootprintName); + + wxString fpDirName = std::get<0>( ret ); + const CFB::COMPOUND_FILE_ENTRY* footprintStream = std::get<1>( ret ); if( fpDirName.IsEmpty() ) { @@ -615,7 +619,7 @@ FOOTPRINT* ALTIUM_PCB::ParseFootprint( const ALTIUM_COMPOUND_FILE& altiumLibFile } const std::vector streamName{ fpDirName.ToStdString(), "Data" }; - const CFB::COMPOUND_FILE_ENTRY* footprintData = altiumLibFile.FindStream( streamName ); + const CFB::COMPOUND_FILE_ENTRY* footprintData = altiumLibFile.FindStream( footprintStream, { "Data" } ); if( footprintData == nullptr ) { @@ -635,7 +639,7 @@ FOOTPRINT* ALTIUM_PCB::ParseFootprint( const ALTIUM_COMPOUND_FILE& altiumLibFile const std::vector parametersStreamName{ fpDirName.ToStdString(), "Parameters" }; const CFB::COMPOUND_FILE_ENTRY* parametersData = - altiumLibFile.FindStream( parametersStreamName ); + altiumLibFile.FindStream( footprintStream, { "Parameters" } ); if( parametersData != nullptr ) { @@ -653,10 +657,10 @@ FOOTPRINT* ALTIUM_PCB::ParseFootprint( const ALTIUM_COMPOUND_FILE& altiumLibFile } const std::vector extendedPrimitiveInformationStreamName{ - fpDirName.ToStdString(), "ExtendedPrimitiveInformation", "Data" + "ExtendedPrimitiveInformation", "Data" }; const CFB::COMPOUND_FILE_ENTRY* extendedPrimitiveInformationData = - altiumLibFile.FindStream( extendedPrimitiveInformationStreamName ); + altiumLibFile.FindStream( footprintStream, extendedPrimitiveInformationStreamName ); if( extendedPrimitiveInformationData != nullptr ) ParseExtendedPrimitiveInformationData( altiumLibFile, extendedPrimitiveInformationData ); diff --git a/thirdparty/compoundfilereader/compoundfilereader.h b/thirdparty/compoundfilereader/compoundfilereader.h index b647a1fe13..b48b8f685b 100644 --- a/thirdparty/compoundfilereader/compoundfilereader.h +++ b/thirdparty/compoundfilereader/compoundfilereader.h @@ -1,23 +1,23 @@ /** Microsoft Compound File (and Property Set) Reader http://en.wikipedia.org/wiki/Compound_File_Binary_Format - + Format specification: MS-CFB: https://msdn.microsoft.com/en-us/library/dd942138.aspx MS-OLEPS: https://msdn.microsoft.com/en-us/library/dd942421.aspx - Note: + Note: 1. For simplification, the code assumes that the target system is little-endian. - + 2. The reader operates the passed buffer in-place. You must keep the input buffer valid when you are using the reader. - + 3. Single thread usage. Example 1: print all streams in a compound file \code CFB::CompoundFileReader reader(buffer, len); - reader.EnumFiles(reader.GetRootEntry(), -1, + reader.EnumFiles(reader.GetRootEntry(), -1, [&](const CFB::COMPOUND_FILE_ENTRY* entry, const CFB::utf16string& dir, int level)->void { bool isDirectory = !reader.IsStream(entry); @@ -99,7 +99,7 @@ struct PROPERTY_SET_STREAM_HDR uint32_t systemIdentifier; unsigned char clsid[16]; uint32_t numPropertySets; - struct + struct { char fmtid[16]; uint32_t offset; @@ -130,7 +130,7 @@ struct helper }; typedef std::basic_string utf16string; -typedef std::function +typedef std::function EnumFilesCallback; class CompoundFileReader @@ -163,7 +163,7 @@ public: m_miniStreamStartSector = root->startSectorLocation; } - /// Get entry (directory or file) by its ID. + /// Get entry (directory or file) by its ID. /// Pass "0" to get the root directory entry. -- This is the start point to navigate the compound file. /// Use the returned object to access child entries. const COMPOUND_FILE_ENTRY* GetEntry(size_t entryID) const @@ -230,7 +230,7 @@ public: private: // Enum entries with same level, including 'entry' itself - void EnumNodes(const COMPOUND_FILE_ENTRY* entry, int currentLevel, int maxLevel, + void EnumNodes(const COMPOUND_FILE_ENTRY* entry, int currentLevel, int maxLevel, const utf16string& dir, EnumFilesCallback callback) const { if (maxLevel > 0 && currentLevel >= maxLevel) @@ -238,7 +238,8 @@ private: if (entry == nullptr) return; - callback(entry, dir, currentLevel + 1); + if( callback(entry, dir, currentLevel + 1) != 0 ) + return; const COMPOUND_FILE_ENTRY* child = GetEntry(entry->childID); if (child != nullptr) @@ -332,8 +333,8 @@ private: { throw FileCorrupted(); } - - + + LocateFinalSector(m_miniStreamStartSector, sector * m_minisectorSize + offset, §or, &offset); return SectorOffsetToAddress(sector, offset); }