diff --git a/src/engine/Operation.cpp b/src/engine/Operation.cpp index ed5d9d3cc6..6c2c8bed76 100644 --- a/src/engine/Operation.cpp +++ b/src/engine/Operation.cpp @@ -226,12 +226,13 @@ ProtoResult Operation::runComputation(const ad_utility::Timer& timer, // _____________________________________________________________________________ CacheValue Operation::runComputationAndPrepareForCache( const ad_utility::Timer& timer, ComputationMode computationMode, - const QueryCacheKey& cacheKey, bool pinned) { + const QueryCacheKey& cacheKey, bool pinned, bool isRoot) { auto& cache = _executionContext->getQueryTreeCache(); auto result = runComputation(timer, computationMode); auto maxSize = - std::min(RuntimeParameters().get<"lazy-result-max-cache-size">(), - cache.getMaxSizeSingleEntry()); + isRoot ? cache.getMaxSizeSingleEntry() + : std::min(RuntimeParameters().get<"lazy-result-max-cache-size">(), + cache.getMaxSizeSingleEntry()); if (canResultBeCached() && !result.isFullyMaterialized() && !unlikelyToFitInCache(maxSize)) { AD_CONTRACT_CHECK(!pinned); @@ -306,9 +307,10 @@ std::shared_ptr Operation::getResult( updateRuntimeInformationOnFailure(timer.msecs()); } }); - auto cacheSetup = [this, &timer, computationMode, &cacheKey, pinResult]() { + auto cacheSetup = [this, &timer, computationMode, &cacheKey, pinResult, + isRoot]() { return runComputationAndPrepareForCache(timer, computationMode, cacheKey, - pinResult); + pinResult, isRoot); }; auto suitedForCache = [](const CacheValue& cacheValue) { diff --git a/src/engine/Operation.h b/src/engine/Operation.h index 893e85ca22..1a3f68b83d 100644 --- a/src/engine/Operation.h +++ b/src/engine/Operation.h @@ -384,7 +384,7 @@ class Operation { CacheValue runComputationAndPrepareForCache(const ad_utility::Timer& timer, ComputationMode computationMode, const QueryCacheKey& cacheKey, - bool pinned); + bool pinned, bool isRoot); // Create and store the complete runtime information for this operation after // it has either been successfully computed or read from the cache. @@ -454,4 +454,5 @@ class Operation { FRIEND_TEST(Operation, ensureLazyOperationIsCachedIfSmallEnough); FRIEND_TEST(Operation, checkLazyOperationIsNotCachedIfTooLarge); FRIEND_TEST(Operation, checkLazyOperationIsNotCachedIfUnlikelyToFitInCache); + FRIEND_TEST(Operation, checkMaxCacheSizeIsComputedCorrectly); }; diff --git a/test/OperationTest.cpp b/test/OperationTest.cpp index 2afbe38a83..d670544dab 100644 --- a/test/OperationTest.cpp +++ b/test/OperationTest.cpp @@ -9,6 +9,7 @@ #include "engine/NeutralElementOperation.h" #include "engine/ValuesForTesting.h" #include "global/RuntimeParameters.h" +#include "util/GTestHelpers.h" #include "util/IdTableHelpers.h" #include "util/IndexTestHelpers.h" #include "util/OperationTestHelpers.h" @@ -556,7 +557,7 @@ TEST(Operation, ensureLazyOperationIsCachedIfSmallEnough) { auto cacheValue = valuesForTesting.runComputationAndPrepareForCache( timer, ComputationMode::LAZY_IF_SUPPORTED, makeQueryCacheKey("test"), - false); + false, false); EXPECT_FALSE( qec->getQueryTreeCache().cacheContains(makeQueryCacheKey("test"))); @@ -614,7 +615,7 @@ TEST(Operation, checkLazyOperationIsNotCachedIfTooLarge) { cacheValue = valuesForTesting.runComputationAndPrepareForCache( timer, ComputationMode::LAZY_IF_SUPPORTED, makeQueryCacheKey("test"), - false); + false, false); EXPECT_FALSE( qec->getQueryTreeCache().cacheContains(makeQueryCacheKey("test"))); } @@ -641,7 +642,7 @@ TEST(Operation, checkLazyOperationIsNotCachedIfUnlikelyToFitInCache) { auto cacheValue = valuesForTesting.runComputationAndPrepareForCache( timer, ComputationMode::LAZY_IF_SUPPORTED, makeQueryCacheKey("test"), - false); + false, false); EXPECT_FALSE( qec->getQueryTreeCache().cacheContains(makeQueryCacheKey("test"))); @@ -653,6 +654,51 @@ TEST(Operation, checkLazyOperationIsNotCachedIfUnlikelyToFitInCache) { qec->getQueryTreeCache().cacheContains(makeQueryCacheKey("test"))); } +// _____________________________________________________________________________ +TEST(Operation, checkMaxCacheSizeIsComputedCorrectly) { + auto runTest = [](ad_utility::MemorySize cacheLimit, + ad_utility::MemorySize runtimeParameterLimit, bool isRoot, + ad_utility::MemorySize expectedSize, + ad_utility::source_location sourceLocation = + ad_utility::source_location::current()) { + auto loc = generateLocationTrace(sourceLocation); + auto qec = getQec(); + qec->getQueryTreeCache().clearAll(); + std::vector idTablesVector{}; + idTablesVector.push_back(makeIdTableFromVector({{3, 4}})); + ValuesForTesting valuesForTesting{ + qec, std::move(idTablesVector), {Variable{"?x"}, Variable{"?y"}}, true}; + + ad_utility::MemorySize actualCacheSize; + valuesForTesting.setCacheSizeStorage(&actualCacheSize); + + absl::Cleanup restoreOriginalSize{ + [qec, original = qec->getQueryTreeCache().getMaxSizeSingleEntry()]() { + qec->getQueryTreeCache().setMaxSizeSingleEntry(original); + }}; + qec->getQueryTreeCache().setMaxSizeSingleEntry(cacheLimit); + + auto cleanup = setRuntimeParameterForTest<"lazy-result-max-cache-size">( + runtimeParameterLimit); + + ad_utility::Timer timer{ad_utility::Timer::InitialStatus::Started}; + + auto cacheValue = valuesForTesting.runComputationAndPrepareForCache( + timer, ComputationMode::LAZY_IF_SUPPORTED, makeQueryCacheKey("test"), + false, isRoot); + + EXPECT_EQ(actualCacheSize, expectedSize); + }; + + runTest(10_B, 10_B, true, 10_B); + runTest(10_B, 10_B, false, 10_B); + runTest(10_B, 1_B, false, 1_B); + runTest(1_B, 10_B, false, 1_B); + runTest(10_B, 1_B, true, 10_B); + runTest(1_B, 10_B, true, 1_B); +} + +// _____________________________________________________________________________ TEST(OperationTest, disableCaching) { auto qec = getQec(); qec->getQueryTreeCache().clearAll(); diff --git a/test/engine/ValuesForTesting.h b/test/engine/ValuesForTesting.h index 097ccd9c78..ec9b55cd30 100644 --- a/test/engine/ValuesForTesting.h +++ b/test/engine/ValuesForTesting.h @@ -22,6 +22,7 @@ class ValuesForTesting : public Operation { size_t sizeEstimate_; size_t costEstimate_; bool unlikelyToFitInCache_ = false; + ad_utility::MemorySize* cacheSizeStorage_ = nullptr; public: // Create an operation that has as its result the given `table` and the given @@ -115,9 +116,17 @@ class ValuesForTesting : public Operation { } return {std::move(table), resultSortedOn(), localVocab_.clone()}; } - bool unlikelyToFitInCache(ad_utility::MemorySize) const override { + bool unlikelyToFitInCache(ad_utility::MemorySize cacheSize) const override { + if (cacheSizeStorage_ != nullptr) { + *cacheSizeStorage_ = cacheSize; + } return unlikelyToFitInCache_; } + + void setCacheSizeStorage(ad_utility::MemorySize* cacheSizeStorage) { + cacheSizeStorage_ = cacheSizeStorage; + } + bool supportsLimit() const override { return supportsLimit_; } bool& forceFullyMaterialized() { return forceFullyMaterialized_; }