From d46c8ffc566db527b01cea97afb2f0af5b62c910 Mon Sep 17 00:00:00 2001 From: Brett Simmers Date: Tue, 30 Jan 2018 17:19:38 -0800 Subject: [PATCH] Optimize GC trigger check Summary: Trim the meat of the check down to one comparison by setting m_nextGC to INT64_MAX when the GC is disabled and moving the rds::header() check to resetGC(), which happens just a handful of times per request. A call to checkGC() now only adds two instructions to the hot path. Reviewed By: edwinsmith Differential Revision: D6797196 fbshipit-source-id: bdc3a274d1485505a73439c39e94e556f59f9dad --- hphp/runtime/base/heap-collect.cpp | 21 ++++++++++++--------- hphp/runtime/base/memory-manager-inl.h | 4 ++++ hphp/runtime/base/memory-manager.cpp | 5 +++-- hphp/runtime/base/memory-manager.h | 5 ++++- hphp/runtime/base/program-functions.cpp | 4 +++- 5 files changed, 26 insertions(+), 13 deletions(-) diff --git a/hphp/runtime/base/heap-collect.cpp b/hphp/runtime/base/heap-collect.cpp index 3ce5eae3418..fd3146db7e1 100644 --- a/hphp/runtime/base/heap-collect.cpp +++ b/hphp/runtime/base/heap-collect.cpp @@ -702,7 +702,7 @@ void collectImpl(HeapImpl& heap, const char* phase, GCBits& mark_version) { void MemoryManager::resetGC() { t_req_num = ++g_req_num; t_gc_num = 0; - updateNextGc(); + if (rds::header()) updateNextGc(); } void MemoryManager::resetEagerGC() { @@ -718,12 +718,10 @@ void MemoryManager::requestEagerGC() { } } +NEVER_INLINE void MemoryManager::requestGC() { - if (this->isGCEnabled() && rds::header()) { - if (m_stats.mmUsage > m_nextGc) { - setSurpriseFlag(PendingGCFlag); - } - } + assertx(rds::header()); + setSurpriseFlag(PendingGCFlag); } /* @@ -733,21 +731,26 @@ void MemoryManager::requestGC() { * auxUsage from the heap limit, before our calculations. * * GC will then be triggered the next time we notice mmUsage > m_nextGc - * (see requestGC(), above). + * (see checkGC() in memory-manager-inl.h). */ void MemoryManager::updateNextGc() { + if (!isGCEnabled()) { + m_nextGC = kNoNextGC; + return; + } + auto stats = getStatsCopy(); auto mm_limit = m_usageLimit - stats.auxUsage(); int64_t delta = (mm_limit - stats.mmUsage) * RuntimeOption::EvalGCTriggerPct; delta = std::max(delta, RuntimeOption::EvalGCMinTrigger); - m_nextGc = stats.mmUsage + delta; + m_nextGC = stats.mmUsage + delta; } void MemoryManager::collect(const char* phase) { if (empty()) return; t_req_age = cpu_ns()/1000 - m_req_start_micros; - t_trigger = m_nextGc; + t_trigger = m_nextGC; collectImpl(m_heap, phase, m_mark_version); updateNextGc(); } diff --git a/hphp/runtime/base/memory-manager-inl.h b/hphp/runtime/base/memory-manager-inl.h index 736242c86f4..f6ca5cbe6ae 100644 --- a/hphp/runtime/base/memory-manager-inl.h +++ b/hphp/runtime/base/memory-manager-inl.h @@ -365,6 +365,10 @@ inline void MemoryManager::forceOOM() { } } +inline void MemoryManager::checkGC() { + if (UNLIKELY(m_stats.mmUsage > m_nextGC)) requestGC(); +} + /////////////////////////////////////////////////////////////////////////////// inline bool MemoryManager::empty() const { diff --git a/hphp/runtime/base/memory-manager.cpp b/hphp/runtime/base/memory-manager.cpp index 0adb9452028..92fad833261 100644 --- a/hphp/runtime/base/memory-manager.cpp +++ b/hphp/runtime/base/memory-manager.cpp @@ -116,7 +116,7 @@ MemoryManager::MemoryManager() { m_bypassSlabAlloc = RuntimeOption::DisableSmallAllocator; m_req_start_micros = HPHP::Timer::GetThreadCPUTimeNanos() / 1000; IniSetting::Bind(IniSetting::CORE, IniSetting::PHP_INI_ALL, "zend.enable_gc", - &m_gc_enabled); + &m_gc_enabled); } MemoryManager::~MemoryManager() { @@ -670,7 +670,7 @@ void splitTail(FreelistArray& freelists, void* tail, size_t tailBytes, */ NEVER_INLINE void* MemoryManager::newSlab(size_t nbytes) { refreshStats(); - requestGC(); + checkGC(); storeTail(m_freelists, m_front, (char*)m_limit - (char*)m_front); auto mem = m_heap.allocSlab(m_stats); assert(reinterpret_cast(mem) % kSlabAlign == 0); @@ -1050,6 +1050,7 @@ bool MemoryManager::isGCEnabled() { void MemoryManager::setGCEnabled(bool isGCEnabled) { m_gc_enabled = isGCEnabled; + updateNextGc(); } void MemoryManager::publishStats(const char* name, diff --git a/hphp/runtime/base/memory-manager.h b/hphp/runtime/base/memory-manager.h index 14576acce6f..d7f6be12022 100644 --- a/hphp/runtime/base/memory-manager.h +++ b/hphp/runtime/base/memory-manager.h @@ -1088,6 +1088,7 @@ private: void requestEagerGC(); void resetEagerGC(); + void checkGC(); void requestGC(); ///////////////////////////////////////////////////////////////////////////// @@ -1095,12 +1096,14 @@ private: private: TRACE_SET_MOD(mm); + static auto constexpr kNoNextGC = std::numeric_limits::max(); + void* m_front{nullptr}; void* m_limit{nullptr}; FreelistArray m_freelists; StringDataNode m_strings; // in-place node is head of circular list std::vector m_apc_arrays; - int64_t m_nextGc; // request gc when heap usage reaches this size + int64_t m_nextGC{kNoNextGC}; // request gc when heap usage reaches this size int64_t m_usageLimit; // OOM when m_stats.usage() > m_usageLimit MemoryUsageStats m_stats; HeapImpl m_heap; diff --git a/hphp/runtime/base/program-functions.cpp b/hphp/runtime/base/program-functions.cpp index 92d42c653c8..053a6c28e0d 100644 --- a/hphp/runtime/base/program-functions.cpp +++ b/hphp/runtime/base/program-functions.cpp @@ -2132,7 +2132,9 @@ void hphp_thread_init() { ExtensionRegistry::threadInit(); InitFiniNode::ThreadInit(); - // ensure that there's no request-allocated memory + // Ensure that there's no request-allocated memory. This call must happen at + // least once after RDS has been initialized by ThreadInfo::init(), to ensure + // MemoryManager::resetGC() sets a proper trigger threshold. hphp_memory_cleanup(); } -- 2.11.4.GIT