From b247c4fdeb5b2ee08c4457ad7ff8ce3076c81c5b Mon Sep 17 00:00:00 2001 From: Marco Bubke Date: Wed, 24 Jan 2018 18:06:15 +0100 Subject: [PATCH] Clang: Store UsedDefines in the symbol database We write the new defines in a temporary table and then write every new entry in the usedDefines table, followed by remove of every entry in usedDefines which are not in the temporary table but have a file entry in common with one of the entries in the temporary table. At last we clean up the temporary table. Change-Id: Idf11ce8d17ad9ccef490578059fac08409fcc5d9 Reviewed-by: Ivan Donchevskii --- .../refactoringdatabaseinitializer.h | 2 +- .../source/storagesqlitestatementfactory.h | 40 ++++++++++++++-- .../source/symbolstorage.h | 6 +++ .../refactoringdatabaseinitializer-test.cpp | 4 +- .../storagesqlitestatementfactory-test.cpp | 46 ++++++++++++++++--- tests/unit/unittest/symbolstorage-test.cpp | 16 +++++++ 6 files changed, 99 insertions(+), 15 deletions(-) diff --git a/src/libs/clangsupport/refactoringdatabaseinitializer.h b/src/libs/clangsupport/refactoringdatabaseinitializer.h index 71dfb840a2d..d609d415ac8 100644 --- a/src/libs/clangsupport/refactoringdatabaseinitializer.h +++ b/src/libs/clangsupport/refactoringdatabaseinitializer.h @@ -139,7 +139,7 @@ public: table.addColumn("usedDefineId", Sqlite::ColumnType::Integer, Sqlite::Contraint::PrimaryKey); const Sqlite::Column &sourceIdColumn = table.addColumn("sourceId", Sqlite::ColumnType::Integer); const Sqlite::Column &defineNameColumn = table.addColumn("defineName", Sqlite::ColumnType::Text); - table.addIndex({sourceIdColumn}); + table.addIndex({sourceIdColumn, defineNameColumn}); table.addIndex({defineNameColumn}); table.initialize(database); diff --git a/src/tools/clangrefactoringbackend/source/storagesqlitestatementfactory.h b/src/tools/clangrefactoringbackend/source/storagesqlitestatementfactory.h index 85f99899e9e..f428e142afb 100644 --- a/src/tools/clangrefactoringbackend/source/storagesqlitestatementfactory.h +++ b/src/tools/clangrefactoringbackend/source/storagesqlitestatementfactory.h @@ -41,8 +41,10 @@ public: using WriteStatementType = WriteStatement; StorageSqliteStatementFactory(Database &database) - : database(database) + : transaction(database), + database(database) { + transaction.commit(); } Sqlite::Table createNewSymbolsTable() const @@ -57,9 +59,7 @@ public: table.addIndex({usrColumn, symbolNameColumn}); table.addIndex({symbolIdColumn}); - Sqlite::ImmediateTransaction transaction(database); table.initialize(database); - transaction.commit(); return table; } @@ -76,17 +76,31 @@ public: const Sqlite::Column &sourceIdColumn = table.addColumn("sourceId", Sqlite::ColumnType::Integer); table.addIndex({sourceIdColumn}); - Sqlite::ImmediateTransaction transaction(database); table.initialize(database); - transaction.commit(); + + return table; + } + + Sqlite::Table createNewUsedDefinesTable() const + { + Sqlite::Table table; + table.setName("newUsedDefines"); + table.setUseTemporaryTable(true); + const Sqlite::Column &sourceIdColumn = table.addColumn("sourceId", Sqlite::ColumnType::Integer); + const Sqlite::Column &defineNameColumn = table.addColumn("defineName", Sqlite::ColumnType::Text); + table.addIndex({sourceIdColumn, defineNameColumn}); + + table.initialize(database); return table; } public: + Sqlite::ImmediateTransaction transaction; Database &database; Sqlite::Table newSymbolsTablet{createNewSymbolsTable()}; Sqlite::Table newLocationsTable{createNewLocationsTable()}; + Sqlite::Table newUsedDefineTable{createNewUsedDefinesTable()}; WriteStatement insertSymbolsToNewSymbolsStatement{ "INSERT INTO newSymbols(temporarySymbolId, usr, symbolName) VALUES(?,?,?)", database}; @@ -152,6 +166,22 @@ public: "SELECT compilerArguments FROM projectParts WHERE projectPartId = (SELECT projectPartId FROM projectPartsSources WHERE sourceId = ?)", database }; + WriteStatement insertIntoNewUsedDefinesStatement{ + "INSERT INTO newUsedDefines(sourceId, defineName) VALUES (?,?)", + database + }; + WriteStatement syncNewUsedDefinesStatement{ + "INSERT INTO usedDefines(sourceId, defineName) SELECT sourceId, defineName FROM newUsedDefines WHERE NOT EXISTS (SELECT sourceId FROM usedDefines WHERE usedDefines.sourceId == newUsedDefines.sourceId AND usedDefines.defineName == newUsedDefines.defineName)", + database + }; + WriteStatement deleteOutdatedUsedDefinesStatement{ + "DELETE FROM usedDefines WHERE sourceId IN (SELECT sourceId FROM newUsedDefines) AND NOT EXISTS (SELECT sourceId FROM newUsedDefines WHERE newUsedDefines.sourceId == usedDefines.sourceId AND newUsedDefines.defineName == usedDefines.defineName)", + database + }; + WriteStatement deleteNewUsedDefinesTableStatement{ + "DELETE FROM newUsedDefines", + database + }; }; } // namespace ClangBackEnd diff --git a/src/tools/clangrefactoringbackend/source/symbolstorage.h b/src/tools/clangrefactoringbackend/source/symbolstorage.h index 54183619dcf..9cdeedf964c 100644 --- a/src/tools/clangrefactoringbackend/source/symbolstorage.h +++ b/src/tools/clangrefactoringbackend/source/symbolstorage.h @@ -83,7 +83,13 @@ public: void insertOrUpdateUsedDefines(const UsedDefines &usedDefines) override { + WriteStatement &insertStatement = m_statementFactory.insertIntoNewUsedDefinesStatement; + for (const UsedDefine &usedDefine : usedDefines) + insertStatement.write(usedDefine.filePathId.filePathId, usedDefine.defineName); + m_statementFactory.syncNewUsedDefinesStatement.execute(); + m_statementFactory.deleteOutdatedUsedDefinesStatement.execute(); + m_statementFactory.deleteNewUsedDefinesTableStatement.execute(); } void updateProjectPartSources(Utils::SmallStringView projectPartName, diff --git a/tests/unit/unittest/refactoringdatabaseinitializer-test.cpp b/tests/unit/unittest/refactoringdatabaseinitializer-test.cpp index 8f24b40b87c..51143bcb157 100644 --- a/tests/unit/unittest/refactoringdatabaseinitializer-test.cpp +++ b/tests/unit/unittest/refactoringdatabaseinitializer-test.cpp @@ -108,7 +108,7 @@ TEST_F(RefactoringDatabaseInitializer, AddUsedDefinesTable) InSequence s; EXPECT_CALL(mockDatabase, execute(Eq("CREATE TABLE IF NOT EXISTS usedDefines(usedDefineId INTEGER PRIMARY KEY, sourceId INTEGER, defineName TEXT)"))); - EXPECT_CALL(mockDatabase, execute(Eq("CREATE INDEX IF NOT EXISTS index_usedDefines_sourceId ON usedDefines(sourceId)"))); + EXPECT_CALL(mockDatabase, execute(Eq("CREATE INDEX IF NOT EXISTS index_usedDefines_sourceId_defineName ON usedDefines(sourceId, defineName)"))); EXPECT_CALL(mockDatabase, execute(Eq("CREATE INDEX IF NOT EXISTS index_usedDefines_defineName ON usedDefines(defineName)"))); initializer.createUsedDefinesTable(); @@ -133,7 +133,7 @@ TEST_F(RefactoringDatabaseInitializer, CreateInTheContructor) EXPECT_CALL(mockDatabase, execute(Eq("CREATE INDEX IF NOT EXISTS index_projectPartsSources_sourceId_projectPartId ON projectPartsSources(sourceId, projectPartId)"))); EXPECT_CALL(mockDatabase, execute(Eq("CREATE INDEX IF NOT EXISTS index_projectPartsSources_projectPartId ON projectPartsSources(projectPartId)"))); EXPECT_CALL(mockDatabase, execute(Eq("CREATE TABLE IF NOT EXISTS usedDefines(usedDefineId INTEGER PRIMARY KEY, sourceId INTEGER, defineName TEXT)"))); - EXPECT_CALL(mockDatabase, execute(Eq("CREATE INDEX IF NOT EXISTS index_usedDefines_sourceId ON usedDefines(sourceId)"))); + EXPECT_CALL(mockDatabase, execute(Eq("CREATE INDEX IF NOT EXISTS index_usedDefines_sourceId_defineName ON usedDefines(sourceId, defineName)"))); EXPECT_CALL(mockDatabase, execute(Eq("CREATE INDEX IF NOT EXISTS index_usedDefines_defineName ON usedDefines(defineName)"))); EXPECT_CALL(mockDatabase, commit()); diff --git a/tests/unit/unittest/storagesqlitestatementfactory-test.cpp b/tests/unit/unittest/storagesqlitestatementfactory-test.cpp index 7116aef222c..ff82e537dfa 100644 --- a/tests/unit/unittest/storagesqlitestatementfactory-test.cpp +++ b/tests/unit/unittest/storagesqlitestatementfactory-test.cpp @@ -50,11 +50,9 @@ TEST_F(StorageSqliteStatementFactory, AddNewSymbolsTable) { InSequence s; - EXPECT_CALL(mockDatabase, immediateBegin()); EXPECT_CALL(mockDatabase, execute(Eq("CREATE TEMPORARY TABLE newSymbols(temporarySymbolId INTEGER PRIMARY KEY, symbolId INTEGER, usr TEXT, symbolName TEXT)"))); EXPECT_CALL(mockDatabase, execute(Eq("CREATE INDEX IF NOT EXISTS index_newSymbols_usr_symbolName ON newSymbols(usr, symbolName)"))); EXPECT_CALL(mockDatabase, execute(Eq("CREATE INDEX IF NOT EXISTS index_newSymbols_symbolId ON newSymbols(symbolId)"))); - EXPECT_CALL(mockDatabase, commit()); factory.createNewSymbolsTable(); } @@ -63,24 +61,35 @@ TEST_F(StorageSqliteStatementFactory, AddNewLocationsTable) { InSequence s; - EXPECT_CALL(mockDatabase, immediateBegin()); EXPECT_CALL(mockDatabase, execute(Eq("CREATE TEMPORARY TABLE newLocations(temporarySymbolId INTEGER, symbolId INTEGER, line INTEGER, column INTEGER, sourceId INTEGER)"))); EXPECT_CALL(mockDatabase, execute(Eq("CREATE INDEX IF NOT EXISTS index_newLocations_sourceId ON newLocations(sourceId)"))); - EXPECT_CALL(mockDatabase, commit()); factory.createNewLocationsTable(); } +TEST_F(StorageSqliteStatementFactory, AddNewUsedDefineTable) +{ + InSequence s; + + EXPECT_CALL(mockDatabase, execute(Eq("CREATE TEMPORARY TABLE newUsedDefines(sourceId INTEGER, defineName TEXT)"))); + EXPECT_CALL(mockDatabase, execute(Eq("CREATE INDEX IF NOT EXISTS index_newUsedDefines_sourceId_defineName ON newUsedDefines(sourceId, defineName)"))); + + factory.createNewUsedDefinesTable(); +} + TEST_F(StorageSqliteStatementFactory, AddTablesInConstructor) { - EXPECT_CALL(mockDatabase, immediateBegin()).Times(2); - EXPECT_CALL(mockDatabase, commit()).Times(2); + InSequence s; + EXPECT_CALL(mockDatabase, immediateBegin()); EXPECT_CALL(mockDatabase, execute(Eq("CREATE TEMPORARY TABLE newSymbols(temporarySymbolId INTEGER PRIMARY KEY, symbolId INTEGER, usr TEXT, symbolName TEXT)"))); EXPECT_CALL(mockDatabase, execute(Eq("CREATE INDEX IF NOT EXISTS index_newSymbols_usr_symbolName ON newSymbols(usr, symbolName)"))); EXPECT_CALL(mockDatabase, execute(Eq("CREATE INDEX IF NOT EXISTS index_newSymbols_symbolId ON newSymbols(symbolId)"))); EXPECT_CALL(mockDatabase, execute(Eq("CREATE TEMPORARY TABLE newLocations(temporarySymbolId INTEGER, symbolId INTEGER, line INTEGER, column INTEGER, sourceId INTEGER)"))); EXPECT_CALL(mockDatabase, execute(Eq("CREATE INDEX IF NOT EXISTS index_newLocations_sourceId ON newLocations(sourceId)"))); + EXPECT_CALL(mockDatabase, execute(Eq("CREATE TEMPORARY TABLE newUsedDefines(sourceId INTEGER, defineName TEXT)"))); + EXPECT_CALL(mockDatabase, execute(Eq("CREATE INDEX IF NOT EXISTS index_newUsedDefines_sourceId_defineName ON newUsedDefines(sourceId, defineName)"))); + EXPECT_CALL(mockDatabase, commit()); StatementFactory factory{mockDatabase}; } @@ -175,10 +184,33 @@ TEST_F(StorageSqliteStatementFactory, InsertProjectPartsSources) Eq("INSERT INTO projectPartsSources(projectPartId, sourceId) VALUES (?,?)")); } -TEST_F(StorageSqliteStatementFactory, CompileArgumentsForFileId) +TEST_F(StorageSqliteStatementFactory, GetCompileArgumentsForFileId) { ASSERT_THAT(factory.getCompileArgumentsForFileId.sqlStatement, Eq("SELECT compilerArguments FROM projectParts WHERE projectPartId = (SELECT projectPartId FROM projectPartsSources WHERE sourceId = ?)")); } +TEST_F(StorageSqliteStatementFactory, InsertIntoNewUsedDefines) +{ + ASSERT_THAT(factory.insertIntoNewUsedDefinesStatement.sqlStatement, + Eq("INSERT INTO newUsedDefines(sourceId, defineName) VALUES (?,?)")); +} + +TEST_F(StorageSqliteStatementFactory, SyncNewUsedDefines) +{ + ASSERT_THAT(factory.syncNewUsedDefinesStatement.sqlStatement, + Eq("INSERT INTO usedDefines(sourceId, defineName) SELECT sourceId, defineName FROM newUsedDefines WHERE NOT EXISTS (SELECT sourceId FROM usedDefines WHERE usedDefines.sourceId == newUsedDefines.sourceId AND usedDefines.defineName == newUsedDefines.defineName)")); +} + +TEST_F(StorageSqliteStatementFactory, DeleteUnusedDefines) +{ + ASSERT_THAT(factory.deleteOutdatedUsedDefinesStatement.sqlStatement, + Eq("DELETE FROM usedDefines WHERE sourceId IN (SELECT sourceId FROM newUsedDefines) AND NOT EXISTS (SELECT sourceId FROM newUsedDefines WHERE newUsedDefines.sourceId == usedDefines.sourceId AND newUsedDefines.defineName == usedDefines.defineName)")); +} + +TEST_F(StorageSqliteStatementFactory, DeleteAllInNewUnusedDefines) +{ + ASSERT_THAT(factory.deleteNewUsedDefinesTableStatement.sqlStatement, + Eq("DELETE FROM newUsedDefines")); +} } diff --git a/tests/unit/unittest/symbolstorage-test.cpp b/tests/unit/unittest/symbolstorage-test.cpp index 96a869e8fb6..37766f4953c 100644 --- a/tests/unit/unittest/symbolstorage-test.cpp +++ b/tests/unit/unittest/symbolstorage-test.cpp @@ -74,6 +74,10 @@ protected: MockSqliteReadStatement &getProjectPartId = statementFactory.getProjectPartId; MockSqliteWriteStatement &deleteAllProjectPartsSourcesWithProjectPartId = statementFactory.deleteAllProjectPartsSourcesWithProjectPartId; MockSqliteWriteStatement &insertProjectPartSources = statementFactory.insertProjectPartSources; + MockSqliteWriteStatement &insertIntoNewUsedDefinesStatement = statementFactory.insertIntoNewUsedDefinesStatement; + MockSqliteWriteStatement &syncNewUsedDefinesStatement = statementFactory.syncNewUsedDefinesStatement; + MockSqliteWriteStatement &deleteOutdatedUsedDefinesStatement = statementFactory.deleteOutdatedUsedDefinesStatement; + MockSqliteWriteStatement &deleteNewUsedDefinesTableStatement = statementFactory.deleteNewUsedDefinesTableStatement; SymbolEntries symbolEntries{{1, {"functionUSR", "function"}}, {2, {"function2USR", "function2"}}}; SourceLocationEntries sourceLocations{{1, {1, 3}, {42, 23}, SymbolType::Declaration}, @@ -204,6 +208,18 @@ TEST_F(SymbolStorage, UpdateProjectPartSources) storage.updateProjectPartSources("project", {{1, 1}, {1, 2}}); } +TEST_F(SymbolStorage, InsertOrUpdateUsedDefines) +{ + InSequence sequence; + + EXPECT_CALL(insertIntoNewUsedDefinesStatement, write(TypedEq(42), TypedEq("FOO"))); + EXPECT_CALL(insertIntoNewUsedDefinesStatement, write(TypedEq(43), TypedEq("BAR"))); + EXPECT_CALL(syncNewUsedDefinesStatement, execute()); + EXPECT_CALL(deleteOutdatedUsedDefinesStatement, execute()); + EXPECT_CALL(deleteNewUsedDefinesTableStatement, execute()); + + storage.insertOrUpdateUsedDefines({{"FOO", {1, 42}}, {"BAR", {1, 43}}}); +} }