From 1959bacf0859a496fd27c83d653e6c4e2bee6fc2 Mon Sep 17 00:00:00 2001 From: Jeff Young Date: Sat, 31 Aug 2024 15:04:15 +0100 Subject: [PATCH] Performance. Don't parse PEGTL twice, once for modelType and once for model itself. Also don't do O(n^2) searches on parameter names when reading libraries. And, because it's still too slow, multi-thread it. --- eeschema/sim/sim_model.cpp | 15 ++++- eeschema/sim/sim_model_ngspice.cpp | 27 +++++--- eeschema/sim/sim_model_spice.cpp | 25 ++++---- eeschema/sim/sim_model_spice_fallback.cpp | 16 ++++- eeschema/sim/sim_model_subckt.cpp | 25 ++------ eeschema/sim/sim_model_subckt.h | 5 +- eeschema/sim/spice_library_parser.cpp | 75 ++++++++++++++++------- eeschema/sim/spice_library_parser.h | 4 +- eeschema/sim/spice_model_parser.cpp | 71 +++++++-------------- eeschema/sim/spice_model_parser.h | 16 ++++- qa/tests/spice/test_library_spice.cpp | 10 +-- 11 files changed, 163 insertions(+), 126 deletions(-) diff --git a/eeschema/sim/sim_model.cpp b/eeschema/sim/sim_model.cpp index 0c3cbf80a1..5f428c2434 100644 --- a/eeschema/sim/sim_model.cpp +++ b/eeschema/sim/sim_model.cpp @@ -836,12 +836,21 @@ void SIM_MODEL::doSetParamValue( int aParamIndex, const std::string& aValue ) void SIM_MODEL::SetParamValue( int aParamIndex, const std::string& aValue, SIM_VALUE::NOTATION aNotation ) { - std::string value = aValue; + wxString value( aValue ); + + // PEGTL is slow as shit. Avoid if possible. + static const wxRegEx plainNumber( wxS( "^[0-9.]*$" ) ); + + if( plainNumber.Matches( value ) ) + { + doSetParamValue( aParamIndex, aValue ); + return; + } if( aNotation != SIM_VALUE::NOTATION::SI || aValue.find( ',' ) != std::string::npos ) - value = SIM_VALUE::ConvertNotation( value, aNotation, SIM_VALUE::NOTATION::SI ); + value = SIM_VALUE::ConvertNotation( aValue, aNotation, SIM_VALUE::NOTATION::SI ); - doSetParamValue( aParamIndex, value ); + doSetParamValue( aParamIndex, aValue ); } diff --git a/eeschema/sim/sim_model_ngspice.cpp b/eeschema/sim/sim_model_ngspice.cpp index 1c57689db1..8a7b1eb4a1 100644 --- a/eeschema/sim/sim_model_ngspice.cpp +++ b/eeschema/sim/sim_model_ngspice.cpp @@ -83,13 +83,23 @@ SIM_MODEL_NGSPICE::SIM_MODEL_NGSPICE( TYPE aType ) : int SIM_MODEL_NGSPICE::doFindParam( const std::string& aParamName ) const { - // Special case to allow escaped model parameters (suffixed with "_") - for( int ii = 0; ii < (int) GetParamCount(); ++ii ) { const PARAM& param = GetParam( ii ); - if( param.Matches( aParamName ) || param.Matches( aParamName + "_" ) ) + if( param.Matches( aParamName ) ) + return ii; + } + + // Look for escaped param names as a second pass (as they're less common) + for( int ii = 0; ii < (int) GetParamCount(); ++ii ) + { + const PARAM& param = GetParam( ii ); + + if( !param.info.name.ends_with( '_' ) ) + continue; + + if( param.Matches( aParamName + "_" ) ) return ii; } @@ -111,7 +121,7 @@ void SIM_MODEL_NGSPICE::SetParamFromSpiceCode( const std::string& aParamName, for( int ii = 0; ii < (int) GetParamCount(); ++ii ) { - const PARAM& param = GetParam(ii); + const PARAM& param = GetParam( ii ); if( param.info.isSpiceInstanceParam || param.info.category == PARAM::CATEGORY::SUPERFLUOUS ) continue; @@ -123,15 +133,18 @@ void SIM_MODEL_NGSPICE::SetParamFromSpiceCode( const std::string& aParamName, } } + // Look for escaped param names as a second pass (as they're less common) for( int ii = 0; ii < (int) GetParamCount(); ++ii ) { - const PARAM& param = GetParam(ii); + const PARAM& param = GetParam( ii ); if( param.info.isSpiceInstanceParam || param.info.category == PARAM::CATEGORY::SUPERFLUOUS ) continue; - if( param.info.name.ends_with( "_" ) - && boost::iequals( param.info.name, aParamName + "_" ) ) + if( !param.info.name.ends_with( '_' ) ) + continue; + + if( param.Matches( aParamName + "_" ) ) { SetParamValue( ii, aValue, aNotation ); return; diff --git a/eeschema/sim/sim_model_spice.cpp b/eeschema/sim/sim_model_spice.cpp index 731ac76ccd..4318aae8e9 100644 --- a/eeschema/sim/sim_model_spice.cpp +++ b/eeschema/sim/sim_model_spice.cpp @@ -2,7 +2,7 @@ * This program source code file is part of KiCad, a free EDA CAD application. * * Copyright (C) 2022 Mikolaj Wielgus - * Copyright (C) 2022-2023 KiCad Developers, see AUTHORS.txt for contributors. + * Copyright (C) 2022-2024 KiCad Developers, see AUTHORS.txt for contributors. * * This program is free software; you can redistribute it and/or * modify it under the terms of the GNU General Public License @@ -57,21 +57,24 @@ std::string SPICE_GENERATOR_SPICE::Preview( const SPICE_ITEM& aItem ) const std::unique_ptr SIM_MODEL_SPICE::Create( const SIM_LIBRARY_SPICE& aLibrary, const std::string& aSpiceCode ) { - SIM_MODEL::TYPE modelType = SPICE_MODEL_PARSER::ReadType( aLibrary, aSpiceCode ); - std::unique_ptr model = SIM_MODEL::Create( modelType ); + SIM_MODEL::TYPE modelType = SIM_MODEL::TYPE::NONE; - if( SIM_MODEL_SPICE* spiceModel = dynamic_cast( model.release() ) ) + try { - try + std::unique_ptr root = SPICE_MODEL_PARSER::ParseModel( aSpiceCode ); + + modelType = SPICE_MODEL_PARSER::ReadType( aLibrary, root ); + + if( auto* model = dynamic_cast( SIM_MODEL::Create( modelType ).release() ) ) { - spiceModel->m_spiceModelParser->ReadModel( aLibrary, aSpiceCode ); - return std::unique_ptr( spiceModel ); - } - catch( const IO_ERROR& ) - { - // Fall back to raw spice code + model->m_spiceModelParser->ReadModel( aLibrary, root ); + return std::unique_ptr( model ); } } + catch( const IO_ERROR& ) + { + // Fall back to raw spice code + } // Fall back to raw spice code return std::make_unique( modelType, aSpiceCode ); diff --git a/eeschema/sim/sim_model_spice_fallback.cpp b/eeschema/sim/sim_model_spice_fallback.cpp index 8b8be5734b..0eb05cb40a 100644 --- a/eeschema/sim/sim_model_spice_fallback.cpp +++ b/eeschema/sim/sim_model_spice_fallback.cpp @@ -64,13 +64,23 @@ std::vector SIM_MODEL_SPICE_FALLBACK::GetPinNames() const int SIM_MODEL_SPICE_FALLBACK::doFindParam( const std::string& aParamName ) const { - // Special case to allow escaped model parameters (suffixed with "_") - for( int ii = 0; ii < GetParamCount(); ++ii ) { const SIM_MODEL::PARAM& param = GetParam( ii ); - if( param.Matches( aParamName ) || param.Matches( aParamName + "_" ) ) + if( param.Matches( aParamName ) ) + return ii; + } + + // Look for escaped param names as a second pass (as they're less common) + for( int ii = 0; ii < GetParamCount(); ++ii ) + { + const SIM_MODEL::PARAM& param = GetParam( ii ); + + if( !param.info.name.ends_with( '_' ) ) + continue; + + if( param.Matches( aParamName + "_" ) ) return ii; } diff --git a/eeschema/sim/sim_model_subckt.cpp b/eeschema/sim/sim_model_subckt.cpp index 9828e11512..60bb8287f7 100644 --- a/eeschema/sim/sim_model_subckt.cpp +++ b/eeschema/sim/sim_model_subckt.cpp @@ -2,7 +2,7 @@ * This program source code file is part of KiCad, a free EDA CAD application. * * Copyright (C) 2022 Mikolaj Wielgus - * Copyright (C) 2022 KiCad Developers, see AUTHORS.txt for contributors. + * Copyright (C) 2022-2024 KiCad Developers, see AUTHORS.txt for contributors. * * This program is free software; you can redistribute it and/or * modify it under the terms of the GNU General Public License @@ -26,7 +26,6 @@ #include #include -#include #include @@ -67,26 +66,11 @@ std::vector SPICE_GENERATOR_SUBCKT::CurrentNames( const SPICE_ITEM& void SPICE_MODEL_PARSER_SUBCKT::ReadModel( const SIM_LIBRARY_SPICE& aLibrary, - const std::string& aSpiceCode ) + std::unique_ptr& aParseTree ) { - tao::pegtl::string_input<> in( aSpiceCode, "from_content" ); - std::unique_ptr root; - - try - { - root = tao::pegtl::parse_tree::parse( in ); - } - catch( const tao::pegtl::parse_error& e ) - { - THROW_IO_ERROR( e.what() ); - } - SIM_MODEL_SUBCKT& model = static_cast( m_model ); - for( const auto& node : root->children ) + for( const auto& node : aParseTree->root->children ) { if( node->is_type() ) { @@ -131,7 +115,8 @@ void SPICE_MODEL_PARSER_SUBCKT::ReadModel( const SIM_LIBRARY_SPICE& aLibrary, } } - model.m_spiceCode = aSpiceCode; + if( aParseTree->root->children.size() == 1 ) + model.m_spiceCode = aParseTree->root->children[0]->string(); } diff --git a/eeschema/sim/sim_model_subckt.h b/eeschema/sim/sim_model_subckt.h index b38c2d217a..ca2e22b5f9 100644 --- a/eeschema/sim/sim_model_subckt.h +++ b/eeschema/sim/sim_model_subckt.h @@ -2,7 +2,7 @@ * This program source code file is part of KiCad, a free EDA CAD application. * * Copyright (C) 2022 Mikolaj Wielgus - * Copyright (C) 2022 KiCad Developers, see AUTHORS.txt for contributors. + * Copyright (C) 2022-2024 KiCad Developers, see AUTHORS.txt for contributors. * * This program is free software; you can redistribute it and/or * modify it under the terms of the GNU General Public License @@ -44,7 +44,8 @@ class SPICE_MODEL_PARSER_SUBCKT : public SPICE_MODEL_PARSER public: using SPICE_MODEL_PARSER::SPICE_MODEL_PARSER; - void ReadModel( const SIM_LIBRARY_SPICE& aLibrary, const std::string& aSpiceCode ) override; + void ReadModel( const SIM_LIBRARY_SPICE& aLibrary, + std::unique_ptr& aParseTree ) override; }; diff --git a/eeschema/sim/spice_library_parser.cpp b/eeschema/sim/spice_library_parser.cpp index cc73110ab6..c3c084c5a8 100644 --- a/eeschema/sim/spice_library_parser.cpp +++ b/eeschema/sim/spice_library_parser.cpp @@ -33,6 +33,8 @@ #include #include +#include +#include namespace SIM_LIBRARY_SPICE_PARSER @@ -120,7 +122,7 @@ void SPICE_LIBRARY_PARSER::readFallbacks( const wxString& aFilePath, REPORTER& a { wxString lib = m_library.m_pathResolver( tokenizer.GetNextToken(), aFilePath ); - parseFile( lib, aReporter ); + readFallbacks( lib, aReporter ); } } } @@ -131,7 +133,9 @@ void SPICE_LIBRARY_PARSER::readFallbacks( const wxString& aFilePath, REPORTER& a } -void SPICE_LIBRARY_PARSER::parseFile( const wxString &aFilePath, REPORTER& aReporter ) +void SPICE_LIBRARY_PARSER::parseFile( const wxString &aFilePath, + std::vector>* aModelQueue, + REPORTER& aReporter ) { try { @@ -146,24 +150,7 @@ void SPICE_LIBRARY_PARSER::parseFile( const wxString &aFilePath, REPORTER& aRepo { if( node->is_type() ) { - std::string model = node->string(); - std::string modelName = node->children.at( 0 )->string(); - - try - { - m_library.m_models.push_back( SIM_MODEL_SPICE::Create( m_library, model ) ); - m_library.m_modelNames.emplace_back( modelName ); - } - catch( const IO_ERROR& e ) - { - aReporter.Report( e.What(), RPT_SEVERITY_ERROR ); - } - catch( ... ) - { - aReporter.Report( wxString::Format( _( "Cannot create sim model from %s" ), - model ), - RPT_SEVERITY_ERROR ); - } + aModelQueue->emplace_back( node->children.at( 0 )->string(), node->string() ); } else if( node->is_type() ) { @@ -171,7 +158,7 @@ void SPICE_LIBRARY_PARSER::parseFile( const wxString &aFilePath, REPORTER& aRepo try { - parseFile( lib, aReporter ); + parseFile( lib, aModelQueue, aReporter ); } catch( const IO_ERROR& e ) { @@ -204,6 +191,8 @@ void SPICE_LIBRARY_PARSER::ReadFile( const wxString& aFilePath, REPORTER& aRepor m_library.m_models.clear(); m_library.m_modelNames.clear(); + std::vector> modelQueue; + // Aside from the simulation model editor dialog, about the only data we use from the // complete models are the pin definitions for SUBCKTs. The standard LTSpice "cmp" libraries // (cmp/standard.bjt, cmp/standard.mos, etc.) have copious error which trip up our parser, @@ -211,5 +200,47 @@ void SPICE_LIBRARY_PARSER::ReadFile( const wxString& aFilePath, REPORTER& aRepor if( !m_forceFullParse && aFilePath.Contains( wxS( "/LTspiceXVII/lib/cmp/standard" ) ) ) readFallbacks( aFilePath, aReporter ); else - parseFile( aFilePath, aReporter ); + parseFile( aFilePath, &modelQueue, aReporter ); + + m_library.m_models.reserve( modelQueue.size() ); + m_library.m_modelNames.reserve( modelQueue.size() ); + + for( int ii = 0; ii < (int) modelQueue.size(); ++ii ) + { + m_library.m_models.emplace_back( nullptr ); + m_library.m_modelNames.emplace_back( "" ); + } + + thread_pool& tp = GetKiCadThreadPool(); + + tp.push_loop( modelQueue.size(), + [&]( const int a, const int b ) + { + for( int ii = a; ii < b; ++ii ) + { + std::unique_ptr model; + + try + { + model = SIM_MODEL_SPICE::Create( m_library, modelQueue[ii].second ); + } + catch( const IO_ERROR& e ) + { + aReporter.Report( e.What(), RPT_SEVERITY_ERROR ); + } + catch( ... ) + { + aReporter.Report( wxString::Format( _( "Cannot create sim model from %s" ), + modelQueue[ii].second ), + RPT_SEVERITY_ERROR ); + } + + if( model ) + { + m_library.m_models[ii] = std::move( model ); + m_library.m_modelNames[ii] = modelQueue[ii].first; + } + } + } ); + tp.wait_for_tasks(); } diff --git a/eeschema/sim/spice_library_parser.h b/eeschema/sim/spice_library_parser.h index 3c1b5cc4e0..dea61593a1 100644 --- a/eeschema/sim/spice_library_parser.h +++ b/eeschema/sim/spice_library_parser.h @@ -45,7 +45,9 @@ public: protected: void readFallbacks( const wxString& aFilePath, REPORTER& aReporter ); - void parseFile( const wxString& aFilePath, REPORTER& aReporter ); + void parseFile( const wxString& aFilePath, + std::vector>* aModelQueue, + REPORTER& aReporter ); private: bool m_forceFullParse; diff --git a/eeschema/sim/spice_model_parser.cpp b/eeschema/sim/spice_model_parser.cpp index fbd244779e..a9e1297678 100644 --- a/eeschema/sim/spice_model_parser.cpp +++ b/eeschema/sim/spice_model_parser.cpp @@ -27,20 +27,10 @@ #include #include #include - -#include -#include #include #include -/** - * Flag to enable SPICE model parser debugging output. - * - * @ingroup trace_env_vars - */ -static const wxChar traceSpiceModelParser[] = wxT( "KICAD_SPICE_MODEL_PARSER" ); - namespace SIM_MODEL_SPICE_PARSER { @@ -59,27 +49,26 @@ namespace SIM_MODEL_SPICE_PARSER } -SIM_MODEL::TYPE SPICE_MODEL_PARSER::ReadType( const SIM_LIBRARY_SPICE& aLibrary, - const std::string& aSpiceCode ) +std::unique_ptr SPICE_MODEL_PARSER::ParseModel( const std::string& aSpiceCode ) { - tao::pegtl::string_input<> in( aSpiceCode, "Spice_Code" ); - std::unique_ptr root; + std::unique_ptr parseTree = std::make_unique(); - try - { - root = tao::pegtl::parse_tree::parse - ( in ); - } - catch( const tao::pegtl::parse_error& e ) - { - wxLogTrace( traceSpiceModelParser, wxS( "%s" ), e.what() ); - return SIM_MODEL::TYPE::NONE; - } + parseTree->in = std::make_unique>( aSpiceCode, "Spice_Code" ); - for( const auto& node : root->children ) + parseTree->root = tao::pegtl::parse_tree::parse + ( *parseTree->in ); + + return parseTree; +} + + +SIM_MODEL::TYPE SPICE_MODEL_PARSER::ReadType( const SIM_LIBRARY_SPICE& aLibrary, + std::unique_ptr& aParseTree ) +{ + for( const auto& node : aParseTree->root->children ) { if( node->is_type() ) { @@ -161,28 +150,12 @@ SIM_MODEL::TYPE SPICE_MODEL_PARSER::ReadType( const SIM_LIBRARY_SPICE& aLibrary, void SPICE_MODEL_PARSER::ReadModel( const SIM_LIBRARY_SPICE& aLibrary, - const std::string& aSpiceCode ) + std::unique_ptr& aParseTree ) { // The default behavior is to treat the Spice param=value pairs as the model parameters and // values (for many models the correspondence is not exact, so this function is overridden). - tao::pegtl::string_input<> in( aSpiceCode, "Spice_Code" ); - std::unique_ptr root; - - try - { - root = tao::pegtl::parse_tree::parse - ( in ); - } - catch( tao::pegtl::parse_error& e ) - { - THROW_IO_ERROR( e.what() ); - } - - for( const auto& node : root->children ) + for( const auto& node : aParseTree->root->children ) { if( node->is_type() ) { @@ -232,14 +205,13 @@ void SPICE_MODEL_PARSER::ReadModel( const SIM_LIBRARY_SPICE& aLibrary, } else if( node->is_type() ) { - std::string modelName; std::string paramName; for( const auto& subnode : node->children ) { if( subnode->is_type() ) { - modelName = subnode->string(); + // Do nothing. } else if( subnode->is_type() ) { @@ -267,7 +239,8 @@ void SPICE_MODEL_PARSER::ReadModel( const SIM_LIBRARY_SPICE& aLibrary, } } - m_model.m_spiceCode = aSpiceCode; + if( aParseTree->root->children.size() == 1 ) + m_model.m_spiceCode = aParseTree->root->children[0]->string(); } diff --git a/eeschema/sim/spice_model_parser.h b/eeschema/sim/spice_model_parser.h index 4b6ff1ab7e..475d891423 100644 --- a/eeschema/sim/spice_model_parser.h +++ b/eeschema/sim/spice_model_parser.h @@ -2,7 +2,7 @@ * This program source code file is part of KiCad, a free EDA CAD application. * * Copyright (C) 2022 Mikolaj Wielgus - * Copyright (C) 2022 KiCad Developers, see AUTHORS.txt for contributors. + * Copyright (C) 2022-2024 KiCad Developers, see AUTHORS.txt for contributors. * * This program is free software; you can redistribute it and/or * modify it under the terms of the GNU General Public License @@ -26,21 +26,31 @@ #define SPICE_MODEL_PARSER_H #include +#include class SIM_MODEL_SPICE; class SIM_LIBRARY_SPICE; +struct PARSE_TREE +{ + std::unique_ptr> in; + std::unique_ptr root; +}; + + class SPICE_MODEL_PARSER { public: + static std::unique_ptr ParseModel( const std::string& aSpiceCode ); + static SIM_MODEL::TYPE ReadType( const SIM_LIBRARY_SPICE& aLibrary, - const std::string& aSpiceCode ); + std::unique_ptr& aRoot ); SPICE_MODEL_PARSER( SIM_MODEL_SPICE& aModel ) : m_model( aModel ) {} virtual ~SPICE_MODEL_PARSER() = default; - virtual void ReadModel( const SIM_LIBRARY_SPICE& aLibrary, const std::string& aSpiceCode ); + virtual void ReadModel( const SIM_LIBRARY_SPICE& aLibrary, std::unique_ptr& aRoot ); protected: static SIM_MODEL::TYPE ReadTypeFromSpiceStrings( const std::string& aTypeString, diff --git a/qa/tests/spice/test_library_spice.cpp b/qa/tests/spice/test_library_spice.cpp index c19460383d..12fa46ea70 100644 --- a/qa/tests/spice/test_library_spice.cpp +++ b/qa/tests/spice/test_library_spice.cpp @@ -275,14 +275,14 @@ BOOST_AUTO_TEST_CASE( Diodes ) BOOST_CHECK_EQUAL( modelName, "D1" ); BOOST_CHECK_EQUAL( model.FindParam( "is" )->value, "1.23n" ); BOOST_CHECK_EQUAL( model.FindParam( "n" )->value, "1.23" ); - BOOST_CHECK_EQUAL( model.FindParam( "rs" )->value, "0.789" ); + BOOST_CHECK_EQUAL( model.FindParam( "rs" )->value, ".7890" ); BOOST_CHECK_EQUAL( model.FindParam( "ikf" )->value, "12.34m" ); BOOST_CHECK_EQUAL( model.FindParam( "xti" )->value, "3" ); BOOST_CHECK_EQUAL( model.FindParam( "eg" )->value, "1.23" ); BOOST_CHECK_EQUAL( model.FindParam( "cjo" )->value, "0.9p" ); - BOOST_CHECK_EQUAL( model.FindParam( "m_" )->value, "0.56" ); - BOOST_CHECK_EQUAL( model.FindParam( "vj" )->value, "0.78" ); - BOOST_CHECK_EQUAL( model.FindParam( "fc" )->value, "0.9" ); + BOOST_CHECK_EQUAL( model.FindParam( "m_" )->value, ".56" ); + BOOST_CHECK_EQUAL( model.FindParam( "vj" )->value, ".78" ); + BOOST_CHECK_EQUAL( model.FindParam( "fc" )->value, ".9" ); BOOST_CHECK_EQUAL( model.FindParam( "isr" )->value, "12.34n" ); BOOST_CHECK_EQUAL( model.FindParam( "nr" )->value, "2.345" ); BOOST_CHECK_EQUAL( model.FindParam( "bv" )->value, "100" ); @@ -362,7 +362,7 @@ BOOST_AUTO_TEST_CASE( Diodes ) BOOST_CHECK_EQUAL( model.FindParam( "ikf" )->value, "111.1" ); BOOST_CHECK_EQUAL( model.FindParam( "xti" )->value, "3" ); BOOST_CHECK_EQUAL( model.FindParam( "eg" )->value, "2.2" ); - BOOST_CHECK_EQUAL( model.FindParam( "m_" )->value, "0.3" ); + BOOST_CHECK_EQUAL( model.FindParam( "m_" )->value, ".3" ); break; case 24: