From d70705b75854bcf5d42885e2ca05a8a64f7e828d Mon Sep 17 00:00:00 2001 From: Johannes Kalmbach Date: Fri, 8 Nov 2024 09:36:35 +0100 Subject: [PATCH] Fix the sonarcloud issues and address Hannah's comments. Signed-off-by: Johannes Kalmbach --- src/engine/QueryExecutionContext.h | 6 +++--- src/index/DeltaTriples.cpp | 3 ++- src/index/DeltaTriples.h | 2 +- src/index/LocatedTriples.cpp | 2 +- src/index/LocatedTriples.h | 2 +- test/DeltaTriplesTest.cpp | 15 +++++++++------ 6 files changed, 17 insertions(+), 13 deletions(-) diff --git a/src/engine/QueryExecutionContext.h b/src/engine/QueryExecutionContext.h index 76bfe1181e..de7b5a4f6e 100644 --- a/src/engine/QueryExecutionContext.h +++ b/src/engine/QueryExecutionContext.h @@ -92,8 +92,8 @@ class QueryExecutionContext { [[nodiscard]] const Index& getIndex() const { return _index; } const LocatedTriplesSnapshot& locatedTriplesSnapshot() const { - AD_CORRECTNESS_CHECK(sharedLocatedTriplesSnapshot != nullptr); - return *sharedLocatedTriplesSnapshot; + AD_CORRECTNESS_CHECK(sharedLocatedTriplesSnapshot_ != nullptr); + return *sharedLocatedTriplesSnapshot_; } void clearCacheUnpinnedOnly() { getQueryTreeCache().clearUnpinnedOnly(); } @@ -130,7 +130,7 @@ class QueryExecutionContext { // snapshot of the current (located) delta triples. These can then be used // by the respective query without interfering with further incoming // update operations. - SharedLocatedTriplesSnapshot sharedLocatedTriplesSnapshot{ + SharedLocatedTriplesSnapshot sharedLocatedTriplesSnapshot_{ _index.deltaTriplesManager().getCurrentSnapshot()}; QueryResultCache* const _subtreeCache; // allocators are copied but hold shared state diff --git a/src/index/DeltaTriples.cpp b/src/index/DeltaTriples.cpp index 080b4cf89d..0d2ac3bac9 100644 --- a/src/index/DeltaTriples.cpp +++ b/src/index/DeltaTriples.cpp @@ -196,7 +196,8 @@ DeltaTriplesManager::DeltaTriplesManager(const IndexImpl& index) currentLocatedTriplesSnapshot_{deltaTriples_.rlock()->getSnapshot()} {} // _____________________________________________________________________________ -void DeltaTriplesManager::modify(std::function function) { +void DeltaTriplesManager::modify( + const std::function& function) { // While holding the lock for the underlying `DeltaTriples`, perform the // actual `function` (typically some combination of insert and delete // operations) and (while still holding the lock) update the diff --git a/src/index/DeltaTriples.h b/src/index/DeltaTriples.h index 7edac3a0a5..afe13c7c07 100644 --- a/src/index/DeltaTriples.h +++ b/src/index/DeltaTriples.h @@ -198,7 +198,7 @@ class DeltaTriplesManager { // the current snapshot. Concurrent calls to `modify` will be serialized, and // each call to `getCurrentSnapshot` will either return the snapshot before or // after a modification, but never one of an ongoing modification. - void modify(std::function function); + void modify(const std::function& function); // Return a shared pointer to a deep copy of the current snapshot. This can // be safely used to execute a query without interfering with future updates. diff --git a/src/index/LocatedTriples.cpp b/src/index/LocatedTriples.cpp index 27d9d6bb5e..e898327a9a 100644 --- a/src/index/LocatedTriples.cpp +++ b/src/index/LocatedTriples.cpp @@ -285,7 +285,7 @@ std::ostream& operator<<(std::ostream& os, const std::vector>& v) { } // ____________________________________________________________________________ -bool LocatedTriplesPerBlock::containsTriple(const IdTriple<0> triple, +bool LocatedTriplesPerBlock::containsTriple(const IdTriple<0>& triple, bool shouldExist) const { auto blockContains = [&triple, shouldExist](const LocatedTriples& lt, size_t blockIndex) { diff --git a/src/index/LocatedTriples.h b/src/index/LocatedTriples.h index 6078ce4edd..c1b612a775 100644 --- a/src/index/LocatedTriples.h +++ b/src/index/LocatedTriples.h @@ -169,7 +169,7 @@ class LocatedTriplesPerBlock { // Only used for testing. Return `true` iff a `LocatedTriple` with the given // value for `shouldExist` is contained in any block. - bool containsTriple(const IdTriple<0> triple, bool shouldExist) const; + bool containsTriple(const IdTriple<0>& triple, bool shouldExist) const; // This operator is only for debugging and testing. It returns a // human-readable representation. diff --git a/test/DeltaTriplesTest.cpp b/test/DeltaTriplesTest.cpp index cde7442887..cb481b43b8 100644 --- a/test/DeltaTriplesTest.cpp +++ b/test/DeltaTriplesTest.cpp @@ -382,12 +382,14 @@ TEST_F(DeltaTriplesTest, DeltaTriplesManager) { // Check for several of the thread-exclusive triples that they are // properly contained in the current snapshot. // - // TODO(Hannah): I don't understand the `false` for the second - // `containsTriple`. auto p = deltaTriplesManager.getCurrentSnapshot(); const auto& locatedSPO = p->getLocatedTriplesForPermutation(Permutation::SPO); EXPECT_TRUE(locatedSPO.containsTriple(triplesToInsert.at(1), true)); + // This triple is exclusive to the thread and is inserted and then + // immediately deleted again. The `DeltaTriples` thus only store it as + // deleted. It might be contained in the original input, hence we + // cannot simply drop it. EXPECT_TRUE(locatedSPO.containsTriple(triplesToInsert.at(2), false)); EXPECT_TRUE(locatedSPO.containsTriple(triplesToDelete.at(2), false)); } @@ -409,11 +411,12 @@ TEST_F(DeltaTriplesTest, DeltaTriplesManager) { // Each of the threads above inserts on thread-exclusive triple, deletes one // thread-exclusive triple and inserts one thread-exclusive triple that is - // deleted right after. Additionally, there is one common triple inserted by - // all the threads and one common triple that is deleted by all the threads. + // deleted right after (This triple is stored as deleted in the `DeltaTriples` + // because it might be contained in the original input). Additionally, there + // is one common triple inserted by// all the threads and one common triple + // that is deleted by all the threads. // - // TODO(Hannah): I don't understand why the number of thread-exclusive deleted - // triples is twice that of the thread-exclusive inserted triples. + auto deltaImpl = deltaTriplesManager.deltaTriples_.rlock(); EXPECT_THAT(*deltaImpl, NumTriples(numThreads + 1, 2 * numThreads + 1, 3 * numThreads + 2));