Database libraries performance improvements

Rework caching system and optimize queries


(cherry picked from commit 39a5929f33)

common/database/database_connection.cpp

@@ -44,6 +44,7 @@
 
 #include <database/database_connection.h>
 #include <database/database_cache.h>
+#include <profile.h>
 
 
 const char* const traceDatabase = "KICAD_DATABASE";
@@ -89,7 +90,7 @@ DATABASE_CONNECTION::DATABASE_CONNECTION( const std::string& aDataSourceName,
     m_pass    = aPassword;
     m_timeout = aTimeoutSeconds;
 
-    m_cache = std::make_unique<DATABASE_CACHE>( 10, 1 );
+    init();
 
     if( aConnectNow )
         Connect();
@@ -103,7 +104,7 @@ DATABASE_CONNECTION::DATABASE_CONNECTION( const std::string& aConnectionString,
     m_connectionString = aConnectionString;
     m_timeout          = aTimeoutSeconds;
 
-    m_cache = std::make_unique<DATABASE_CACHE>( 10, 1 );
+    init();
 
     if( aConnectNow )
         Connect();
@@ -117,6 +118,12 @@ DATABASE_CONNECTION::~DATABASE_CONNECTION()
 }
 
 
+void DATABASE_CONNECTION::init()
+{
+    m_cache = std::make_unique<DB_CACHE_TYPE>( 10, 1 );
+}
+
+
 void DATABASE_CONNECTION::SetCacheParams( int aMaxSize, int aMaxAge )
 {
     if( !m_cache )
@@ -159,12 +166,10 @@ bool DATABASE_CONNECTION::Connect()
         return false;
     }
 
+    m_tables.clear();
+
     if( IsConnected() )
-    {
-        syncTables();
-        cacheColumns();
         getQuoteChar();
-    }
 
     return IsConnected();
 }
@@ -201,54 +206,32 @@ bool DATABASE_CONNECTION::IsConnected() const
 }
 
 
-bool DATABASE_CONNECTION::syncTables()
+bool DATABASE_CONNECTION::CacheTableInfo( const std::string& aTable,
+                                          const std::set<std::string>& aColumns )
 {
     if( !m_conn )
         return false;
 
-    m_tables.clear();
-
     try
     {
         nanodbc::catalog catalog( *m_conn );
-        nanodbc::catalog::tables tables = catalog.find_tables();
+        nanodbc::catalog::tables tables = catalog.find_tables( fromUTF8( aTable ) );
 
-        while( tables.next() )
+        tables.next();
-        {
         std::string key = toUTF8( tables.table_name() );
         m_tables[key] = toUTF8( tables.table_type() );
-        }
-    }
-    catch( nanodbc::database_error& e )
-    {
-        m_lastError = e.what();
-        wxLogTrace( traceDatabase, wxT( "Exception while syncing tables: %s" ), m_lastError );
-        return false;
-    }
 
-    return true;
-}
-
-
-bool DATABASE_CONNECTION::cacheColumns()
-{
-    if( !m_conn )
-        return false;
-
-    m_columnCache.clear();
-
-    for( const auto& tableIter : m_tables )
-    {
         try
         {
-            nanodbc::catalog catalog( *m_conn );
             nanodbc::catalog::columns columns =
-                    catalog.find_columns( NANODBC_TEXT( "" ), fromUTF8( tableIter.first ) );
+                    catalog.find_columns( NANODBC_TEXT( "" ), tables.table_name() );
 
             while( columns.next() )
             {
                 std::string columnKey = toUTF8( columns.column_name() );
-                m_columnCache[tableIter.first][columnKey] = columns.data_type();
+
+                if( aColumns.count( columnKey ) )
+                    m_columnCache[key][columnKey] = columns.data_type();
             }
 
         }
@@ -256,10 +239,16 @@ bool DATABASE_CONNECTION::cacheColumns()
         {
             m_lastError = e.what();
             wxLogTrace( traceDatabase, wxT( "Exception while syncing columns for table %s: %s" ),
-                        tableIter.first, m_lastError );
+                        key, m_lastError );
             return false;
         }
     }
+    catch( nanodbc::database_error& e )
+    {
+        m_lastError = e.what();
+        wxLogTrace( traceDatabase, wxT( "Exception while caching table info: %s" ), m_lastError );
+        return false;
+    }
 
     return true;
 }
@@ -291,6 +280,35 @@ bool DATABASE_CONNECTION::getQuoteChar()
 }
 
 
+std::string DATABASE_CONNECTION::columnsFor( const std::string& aTable )
+{
+    if( !m_columnCache.count( aTable ) )
+    {
+        wxLogTrace( traceDatabase, wxT( "columnsFor: requested table %s missing from cache!" ),
+                    aTable );
+        return "*";
+    }
+
+    if( m_columnCache[aTable].empty() )
+    {
+        wxLogTrace( traceDatabase, wxT( "columnsFor: requested table %s has no columns mapped!" ),
+                    aTable );
+        return "*";
+    }
+
+    std::string ret;
+
+    for( const auto& [ columnName, columnType ] : m_columnCache[aTable] )
+        ret += fmt::format( "{}{}{}, ", m_quoteChar, columnName, m_quoteChar );
+
+    // strip tailing ', '
+    ret.resize( ret.length() - 2 );
+
+    return ret;
+}
+
+//next step, make SelectOne take from the SelectAll cache if the SelectOne cache is missing.
+//To do this, need to build a map of PK->ROW for the cache result.
 bool DATABASE_CONNECTION::SelectOne( const std::string& aTable,
                                      const std::pair<std::string, std::string>& aWhere,
                                      DATABASE_CONNECTION::ROW& aResult )
@@ -311,6 +329,18 @@ bool DATABASE_CONNECTION::SelectOne( const std::string& aTable,
     }
 
     const std::string& tableName = tableMapIter->first;
+    DB_CACHE_TYPE::CACHE_VALUE cacheEntry;
+
+    if( m_cache->Get( tableName, cacheEntry ) )
+    {
+        if( cacheEntry.count( aWhere.second ) )
+        {
+            wxLogTrace( traceDatabase, wxT( "SelectOne: `%s` with parameter `%s` - cache hit" ),
+                        tableName, aWhere.second );
+            aResult = cacheEntry.at( aWhere.second );
+            return true;
+        }
+    }
 
     if( !m_columnCache.count( tableName ) )
     {
@@ -332,19 +362,15 @@ bool DATABASE_CONNECTION::SelectOne( const std::string& aTable,
 
     std::string cacheKey = fmt::format( "{}{}{}", tableName, columnName, aWhere.second );
 
-    std::string queryStr = fmt::format( "SELECT * FROM {}{}{} WHERE {}{}{} = ?",
+    std::string queryStr = fmt::format( "SELECT {} FROM {}{}{} WHERE {}{}{} = ?",
+                                        columnsFor( tableName ),
                                         m_quoteChar, tableName, m_quoteChar,
                                         m_quoteChar, columnName, m_quoteChar );
 
     nanodbc::statement statement( *m_conn );
     nanodbc::string query = fromUTF8( queryStr );
 
-    if( m_cache->Get( cacheKey, aResult ) )
+    PROF_TIMER timer;
-    {
-        wxLogTrace( traceDatabase, wxT( "SelectOne: `%s` with parameter `%s` - cache hit" ),
-                    toUTF8( query ), aWhere.second );
-        return true;
-    }
 
     try
     {
@@ -384,14 +410,17 @@ bool DATABASE_CONNECTION::SelectOne( const std::string& aTable,
         return false;
     }
 
+    timer.Stop();
+
+
     if( !results.first() )
     {
         wxLogTrace( traceDatabase, wxT( "SelectOne: no results returned from query" ) );
         return false;
     }
 
-    wxLogTrace( traceDatabase, wxT( "SelectOne: %ld results returned from query" ),
+    wxLogTrace( traceDatabase, wxT( "SelectOne: %ld results returned from query in %0.1f ms" ),
-                results.rows() );
+                results.rows(), timer.msecs() );
 
     aResult.clear();
 
@@ -411,13 +440,12 @@ bool DATABASE_CONNECTION::SelectOne( const std::string& aTable,
         return false;
     }
 
-    m_cache->Put( cacheKey, aResult );
-
     return true;
 }
 
 
-bool DATABASE_CONNECTION::SelectAll( const std::string& aTable, std::vector<ROW>& aResults )
+bool DATABASE_CONNECTION::SelectAll( const std::string& aTable, const std::string& aKey,
+                                     std::vector<ROW>& aResults )
 {
     if( !m_conn )
     {
@@ -434,13 +462,27 @@ bool DATABASE_CONNECTION::SelectAll( const std::string& aTable, std::vector<ROW>
         return false;
     }
 
+    DB_CACHE_TYPE::CACHE_VALUE cacheEntry;
+
+    if( m_cache->Get( aTable, cacheEntry ) )
+    {
+        wxLogTrace( traceDatabase, wxT( "SelectAll: `%s` - cache hit" ), aTable );
+
+        for( auto &[ key, row ] : cacheEntry )
+            aResults.emplace_back( row );
+
+        return true;
+    }
+
     nanodbc::statement statement( *m_conn );
 
-    nanodbc::string query = fromUTF8( fmt::format( "SELECT * FROM {}{}{}", m_quoteChar,
+    nanodbc::string query = fromUTF8( fmt::format( "SELECT {} FROM {}{}{}", columnsFor( aTable ),
-                                                   tableMapIter->first, m_quoteChar ) );
+                                                   m_quoteChar, aTable, m_quoteChar ) );
 
     wxLogTrace( traceDatabase, wxT( "SelectAll: `%s`" ), toUTF8( query ) );
 
+    PROF_TIMER timer;
+
     try
     {
         statement.prepare( query );
@@ -475,6 +517,8 @@ bool DATABASE_CONNECTION::SelectAll( const std::string& aTable, std::vector<ROW>
         return false;
     }
 
+    timer.Stop();
+
     try
     {
         while( results.next() )
@@ -498,5 +542,17 @@ bool DATABASE_CONNECTION::SelectAll( const std::string& aTable, std::vector<ROW>
         return false;
     }
 
+    wxLogTrace( traceDatabase, wxT( "SelectAll from %s completed in %0.1f ms" ), aTable,
+                timer.msecs() );
+
+    for( const ROW& row : aResults )
+    {
+        wxASSERT( row.count( aKey ) );
+        std::string keyStr = std::any_cast<std::string>( row.at( aKey ) );
+        cacheEntry[keyStr] = row;
+    }
+
+    m_cache->Put( aTable, cacheEntry );
+
     return true;
 }

eeschema/sch_plugins/database/sch_database_plugin.cpp

@@ -73,7 +73,7 @@ void SCH_DATABASE_PLUGIN::EnumerateSymbolLib( std::vector<LIB_SYMBOL*>& aSymbolL
     {
         std::vector<DATABASE_CONNECTION::ROW> results;
 
-        if( !m_conn->SelectAll( table.table, results ) )
+        if( !m_conn->SelectAll( table.table, table.key_col, results ) )
         {
             if( !m_conn->GetLastError().empty() )
             {
@@ -287,6 +287,26 @@ void SCH_DATABASE_PLUGIN::ensureConnection()
             THROW_IO_ERROR( msg );
         }
 
+        for( const DATABASE_LIB_TABLE& tableIter : m_settings->m_Tables )
+        {
+            std::set<std::string> columns;
+
+            columns.insert( tableIter.key_col );
+            columns.insert( tableIter.footprints_col );
+            columns.insert( tableIter.symbols_col );
+
+            columns.insert( tableIter.properties.description );
+            columns.insert( tableIter.properties.footprint_filters );
+            columns.insert( tableIter.properties.keywords );
+            columns.insert( tableIter.properties.exclude_from_bom );
+            columns.insert( tableIter.properties.exclude_from_board );
+
+            for( const DATABASE_FIELD_MAPPING& field : tableIter.fields )
+                columns.insert( field.column );
+
+            m_conn->CacheTableInfo( tableIter.table, columns );
+        }
+
         m_conn->SetCacheParams( m_settings->m_Cache.max_size, m_settings->m_Cache.max_age );
     }
 }

include/database/database_cache.h

@@ -29,17 +29,24 @@
 #include <database/database_connection.h>
 
 
+template<typename CacheValueType>
 class DATABASE_CACHE
 {
 public:
-    typedef std::pair<std::string, std::pair<time_t, DATABASE_CONNECTION::ROW>> CACHE_ENTRY;
+    typedef std::pair<std::string, std::pair<time_t, CacheValueType>> CACHE_ENTRY;
+
+    typedef std::unordered_map<std::string, typename std::list<CACHE_ENTRY>::iterator> CACHE_TYPE;
+
+    typedef typename CACHE_TYPE::const_iterator CACHE_CITER;
+
+    typedef CacheValueType CACHE_VALUE;
 
     DATABASE_CACHE( size_t aMaxSize, time_t aMaxAge ) :
             m_maxSize( aMaxSize ),
             m_maxAge( aMaxAge )
     {}
 
-    void Put( const std::string& aQuery, const DATABASE_CONNECTION::ROW& aResult )
+    void Put( const std::string& aQuery, const CacheValueType& aResult )
     {
         auto it = m_cache.find( aQuery );
 
@@ -65,7 +72,7 @@ public:
         }
     }
 
-    bool Get( const std::string& aQuery, DATABASE_CONNECTION::ROW& aResult )
+    bool Get( const std::string& aQuery, CacheValueType& aResult )
     {
         auto it = m_cache.find( aQuery );
 
@@ -94,7 +101,7 @@ private:
     size_t m_maxSize;
     time_t m_maxAge;
     std::list<CACHE_ENTRY> m_cacheMru;
-    std::unordered_map<std::string, std::list<CACHE_ENTRY>::iterator> m_cache;
+    CACHE_TYPE m_cache;
 };
 
 #endif //KICAD_DATABASE_CACHE_H

include/database/database_connection.h

@@ -24,8 +24,11 @@
 #include <any>
 #include <map>
 #include <memory>
+#include <set>
 #include <vector>
 
+#include <database/database_cache.h>
+
 extern const char* const traceDatabase;
 
 
@@ -34,8 +37,6 @@ namespace nanodbc
     class connection;
 }
 
-class DATABASE_CACHE;
-
 
 class DATABASE_CONNECTION
 {
@@ -62,6 +63,8 @@ public:
 
     bool IsConnected() const;
 
+    bool CacheTableInfo( const std::string& aTable, const std::set<std::string>& aColumns );
+
     /**
      * Retrieves a single row from a database table.  Table and column names are cached when the
      * connection is created, so schema changes to the database won't be recognized unless the
@@ -77,20 +80,22 @@ public:
     /**
      * Retrieves all rows from a database table.
      * @param aTable the name of a table in the database
+     * @param aKey holds the column name of the primary key used for caching results
      * @param aResults will be filled with all rows in the table
      * @return true if the query succeeded and at least one ROW was found, false otherwise
      */
-    bool SelectAll( const std::string& aTable, std::vector<ROW>& aResults );
+    bool SelectAll( const std::string& aTable, const std::string& aKey,
+                    std::vector<ROW>& aResults );
 
     std::string GetLastError() const { return m_lastError; }
 
 private:
-    bool syncTables();
+    void init();
-
-    bool cacheColumns();
 
     bool getQuoteChar();
 
+    std::string columnsFor( const std::string& aTable );
+
     std::unique_ptr<nanodbc::connection> m_conn;
 
     std::string m_dsn;
@@ -109,7 +114,9 @@ private:
 
     char m_quoteChar;
 
-    std::unique_ptr<DATABASE_CACHE> m_cache;
+    typedef DATABASE_CACHE<std::map<std::string, ROW>> DB_CACHE_TYPE;
+
+    std::unique_ptr<DB_CACHE_TYPE> m_cache;
 };
 
 #endif //KICAD_DATABASE_CONNECTION_H