diff --git a/presto-native-execution/presto_cpp/main/LinuxMemoryChecker.cpp b/presto-native-execution/presto_cpp/main/LinuxMemoryChecker.cpp index 3d9aa0ac77c71..a344d24aadd8f 100644 --- a/presto-native-execution/presto_cpp/main/LinuxMemoryChecker.cpp +++ b/presto-native-execution/presto_cpp/main/LinuxMemoryChecker.cpp @@ -63,7 +63,7 @@ class LinuxMemoryChecker : public PeriodicMemoryChecker { ~LinuxMemoryChecker() override {} int64_t getUsedMemory() { - return systemUsedMemoryBytes(); + return systemUsedMemoryBytes(/*fetchFresh=*/true); } void setStatFile(std::string statFile) { @@ -180,7 +180,7 @@ class LinuxMemoryChecker : public PeriodicMemoryChecker { // value. It may be better than what we currently use. For // consistency we will match cgroup V1 and change if // necessary. - int64_t systemUsedMemoryBytes() override { + void loadSystemMemoryUsage() override { size_t memAvailable = 0; size_t memTotal = 0; size_t inactiveAnon = 0; @@ -207,7 +207,7 @@ class LinuxMemoryChecker : public PeriodicMemoryChecker { // Unit is in bytes. const auto memBytes = inactiveAnon + activeAnon; cachedSystemUsedMemoryBytes_ = memBytes; - return memBytes; + return; } // Last resort use host machine info. @@ -231,7 +231,6 @@ class LinuxMemoryChecker : public PeriodicMemoryChecker { const auto memBytes = (memAvailable && memTotal) ? memTotal - memAvailable : 0; cachedSystemUsedMemoryBytes_ = memBytes; - return memBytes; } int64_t mallocBytes() const override { diff --git a/presto-native-execution/presto_cpp/main/PeriodicMemoryChecker.cpp b/presto-native-execution/presto_cpp/main/PeriodicMemoryChecker.cpp index ece456c6c7ebd..a4eadbb4d1d18 100644 --- a/presto-native-execution/presto_cpp/main/PeriodicMemoryChecker.cpp +++ b/presto-native-execution/presto_cpp/main/PeriodicMemoryChecker.cpp @@ -71,6 +71,7 @@ void PeriodicMemoryChecker::start() { scheduler_->setThreadName("MemoryCheckerThread"); scheduler_->addFunction( [&]() { + loadSystemMemoryUsage(); periodicCb(); if (config_.mallocMemHeapDumpEnabled) { maybeDumpHeap(); @@ -92,6 +93,13 @@ void PeriodicMemoryChecker::stop() { scheduler_.reset(); } +int64_t PeriodicMemoryChecker::systemUsedMemoryBytes(bool fetchFresh) { + if (fetchFresh) { + loadSystemMemoryUsage(); + } + return cachedSystemUsedMemoryBytes_; +} + std::string PeriodicMemoryChecker::createHeapDumpFilePath() const { const size_t now = velox::getCurrentTimeMs() / 1000; // Format as follow: @@ -210,7 +218,9 @@ void PeriodicMemoryChecker::pushbackMemory() { RECORD_HISTOGRAM_METRIC_VALUE( kCounterMemoryPushbackLatencyMs, latencyUs / 1000); const auto actualFreedBytes = std::max( - 0, static_cast(currentMemBytes) - systemUsedMemoryBytes()); + 0, + static_cast(currentMemBytes) - + systemUsedMemoryBytes(/*fetchFresh=*/true)); RECORD_HISTOGRAM_METRIC_VALUE( kCounterMemoryPushbackExpectedReductionBytes, freedBytes); RECORD_HISTOGRAM_METRIC_VALUE( diff --git a/presto-native-execution/presto_cpp/main/PeriodicMemoryChecker.h b/presto-native-execution/presto_cpp/main/PeriodicMemoryChecker.h index a1b3238458872..48b8ea5ab9236 100644 --- a/presto-native-execution/presto_cpp/main/PeriodicMemoryChecker.h +++ b/presto-native-execution/presto_cpp/main/PeriodicMemoryChecker.h @@ -76,15 +76,14 @@ class PeriodicMemoryChecker { /// Stops the 'PeriodicMemoryChecker'. virtual void stop(); - /// Returns the last known cached 'current' system memory usage in bytes. - int64_t cachedSystemUsedMemoryBytes() const { - return cachedSystemUsedMemoryBytes_; - } + /// Returns the last known cached 'current' system memory usage in bytes. If + /// 'fetchFresh' is true, retrieves and returns the current system memory usage. + /// The returned value is used to compare with 'Config::systemMemLimitBytes'. + int64_t systemUsedMemoryBytes(bool fetchFresh = false); protected: - /// Fetches and returns current system memory usage in bytes. - /// The returned value is used to compare with 'Config::systemMemLimitBytes'. - virtual int64_t systemUsedMemoryBytes() = 0; + /// Fetches current system memory usage in bytes and stores it in the cache. + virtual void loadSystemMemoryUsage() = 0; /// Returns current bytes allocated by malloc. The returned value is used to /// compare with 'Config::mallocBytesUsageDumpThreshold' diff --git a/presto-native-execution/presto_cpp/main/PrestoServer.cpp b/presto-native-execution/presto_cpp/main/PrestoServer.cpp index 988895a824109..65e60cab65e76 100644 --- a/presto-native-execution/presto_cpp/main/PrestoServer.cpp +++ b/presto-native-execution/presto_cpp/main/PrestoServer.cpp @@ -1530,7 +1530,7 @@ void PrestoServer::checkOverload() { systemConfig->workerOverloadedThresholdMemGb() * 1024 * 1024 * 1024; if (overloadedThresholdMemBytes > 0) { const auto currentUsedMemoryBytes = (memoryChecker_ != nullptr) - ? memoryChecker_->cachedSystemUsedMemoryBytes() + ? memoryChecker_->systemUsedMemoryBytes() : 0; const bool memOverloaded = (currentUsedMemoryBytes > overloadedThresholdMemBytes); diff --git a/presto-native-execution/presto_cpp/main/tests/PeriodicMemoryCheckerTest.cpp b/presto-native-execution/presto_cpp/main/tests/PeriodicMemoryCheckerTest.cpp index 445bc82ee9dc4..4c625412cba37 100644 --- a/presto-native-execution/presto_cpp/main/tests/PeriodicMemoryCheckerTest.cpp +++ b/presto-native-execution/presto_cpp/main/tests/PeriodicMemoryCheckerTest.cpp @@ -34,10 +34,11 @@ class PeriodicMemoryCheckerTest : public testing::Test { std::function&& periodicCb = nullptr, std::function&& heapDumpCb = nullptr) : PeriodicMemoryChecker(config), - systemUsedMemoryBytes_(systemUsedMemoryBytes), mallocBytes_(mallocBytes), periodicCb_(std::move(periodicCb)), - heapDumpCb_(std::move(heapDumpCb)) {} + heapDumpCb_(std::move(heapDumpCb)) { + cachedSystemUsedMemoryBytes_ = systemUsedMemoryBytes; + } ~TestPeriodicMemoryChecker() override {} @@ -46,9 +47,7 @@ class PeriodicMemoryCheckerTest : public testing::Test { } protected: - int64_t systemUsedMemoryBytes() override { - return systemUsedMemoryBytes_; - } + void loadSystemMemoryUsage() override {} int64_t mallocBytes() const override { return mallocBytes_; @@ -70,7 +69,6 @@ class PeriodicMemoryCheckerTest : public testing::Test { void removeDumpFile(const std::string& filePath) const override {} private: - int64_t systemUsedMemoryBytes_{0}; int64_t mallocBytes_{0}; std::function periodicCb_; std::function heapDumpCb_;