Skip to content

Commit

Permalink
Ensure MemoryUsageStats.maxBytes is always valid
Browse files Browse the repository at this point in the history
In SmartAllocator.alloc() we execute if (hhvm) .. if (m_stats.maxBytes > 0)
on every smart allocation, which is a very hot path.  We can eliminte this
check by just ensuring maxBytes is always valid (use MAX_INT64 instead of
0 or -1 to suppress the check).

Perflab says this saves about 1% CPU time, just above the noise
margin, but CPU instructions are measurably down and it's obviously
good to streamline the allocator hot path.
  • Loading branch information
edwinsmith authored and sgolemon committed Jul 28, 2012
1 parent 17dfa43 commit aa63830
Show file tree
Hide file tree
Showing 5 changed files with 19 additions and 8 deletions.
4 changes: 2 additions & 2 deletions src/runtime/base/execution_context.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ BaseExecutionContext::BaseExecutionContext() :
NEAR_FIELD_INIT
#endif
m_transport(NULL),
m_maxMemory(RuntimeOption::RequestMemoryMaxBytes),
m_maxTime(RuntimeOption::RequestTimeoutSeconds),
m_cwd(Process::CurrentWorkingDirectory),
m_out(NULL), m_implicitFlush(false), m_protectedLevel(0),
Expand All @@ -69,7 +68,7 @@ BaseExecutionContext::BaseExecutionContext() :
m_lastErrorNum(0), m_logErrors(false), m_throwAllErrors(false),
m_vhost(NULL) {

MemoryManager::TheMemoryManager()->getStats().maxBytes = m_maxMemory;
setRequestMemoryMaxBytes(RuntimeOption::RequestMemoryMaxBytes);
m_include_paths = Array::Create();
for (unsigned int i = 0; i < RuntimeOption::IncludeSearchPaths.size(); ++i) {
m_include_paths.append(String(RuntimeOption::IncludeSearchPaths[i]));
Expand Down Expand Up @@ -219,6 +218,7 @@ void BaseExecutionContext::setContentType(CStrRef mimetype, CStrRef charset) {
}

void BaseExecutionContext::setRequestMemoryMaxBytes(int64 max) {
ASSERT(max > 0);
m_maxMemory = max;
MemoryManager::TheMemoryManager()->getStats().maxBytes = m_maxMemory;
}
Expand Down
2 changes: 1 addition & 1 deletion src/runtime/base/memory/memory_manager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ MemoryManager::MemoryManager() : m_enabled(false) {
threadStats(m_allocated, m_deallocated, m_cactive, m_cactiveLimit);
#endif
resetStats();
m_stats.maxBytes = 0;
m_stats.maxBytes = INT64_MAX;
}

void MemoryManager::resetStats() {
Expand Down
3 changes: 2 additions & 1 deletion src/runtime/base/memory/memory_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,8 @@ class MemoryManager : boost::noncopyable {
if (m_stats.usage > m_stats.peakUsage) {
// NOTE: the peak memory usage monotonically increases, so there cannot
// be a second OOM exception in one request.
if (m_stats.maxBytes > 0 && m_stats.peakUsage <= m_stats.maxBytes &&
ASSERT(m_stats.maxBytes > 0);
if (m_stats.peakUsage <= m_stats.maxBytes &&
m_stats.usage > m_stats.maxBytes) {
refreshStatsHelperExceeded();
}
Expand Down
7 changes: 5 additions & 2 deletions src/runtime/base/memory/smart_allocator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -169,14 +169,17 @@ SmartAllocatorImpl::~SmartAllocatorImpl() {
HOT_FUNC
void *SmartAllocatorImpl::alloc() {
ASSERT(m_stats);
MemoryUsageStats* stats = m_stats;
// Just update the usage, while the peakUsage is maintained by
// FrameInjection.
m_stats->usage += m_itemSize;
int64 usage = stats->usage + m_itemSize;
stats->usage = usage;
if (hhvm) {
// It's possible that this simplified check will trip later than
// it should in a perfect world but it's cheaper than a full call
// to refreshStats on every alloc().
if (m_stats->maxBytes > 0 && UNLIKELY(m_stats->usage > m_stats->maxBytes)) {
ASSERT(stats->maxBytes > 0);
if (UNLIKELY(usage > stats->maxBytes)) {
MemoryManager::TheMemoryManager()->refreshStats();
}
}
Expand Down
11 changes: 9 additions & 2 deletions src/runtime/base/runtime_option.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,13 @@
| [email protected] so we can mail you a copy immediately. |
+----------------------------------------------------------------------+
*/
// Get SIZE_MAX definition. Do this before including any other files, to make
// sure that this is the first place that stdint.h is included.
#ifndef __STDC_LIMIT_MACROS
#define __STDC_LIMIT_MACROS
#endif
#define __STDC_LIMIT_MACROS
#include <stdint.h>

#include <runtime/base/runtime_option.h>
#include <runtime/base/type_conversions.h>
Expand Down Expand Up @@ -107,7 +114,7 @@ bool RuntimeOption::PageletServerThreadDropStack = false;
int RuntimeOption::FiberCount = 1;
int RuntimeOption::RequestTimeoutSeconds = 0;
size_t RuntimeOption::ServerMemoryHeadRoom = 0;
int64 RuntimeOption::RequestMemoryMaxBytes = -1;
int64 RuntimeOption::RequestMemoryMaxBytes = INT64_MAX;
int64 RuntimeOption::ImageMemoryMaxBytes = 0;
int RuntimeOption::ResponseQueueCount;
int RuntimeOption::ServerGracefulShutdownWait;
Expand Down Expand Up @@ -665,7 +672,7 @@ void RuntimeOption::Load(Hdf &config, StringVec *overwrites /* = NULL */,
ServerStatCache = server["StatCache"].getBool(true);
RequestTimeoutSeconds = server["RequestTimeoutSeconds"].getInt32(0);
ServerMemoryHeadRoom = server["MemoryHeadRoom"].getInt64(0);
RequestMemoryMaxBytes = server["RequestMemoryMaxBytes"].getInt64(-1);
RequestMemoryMaxBytes = server["RequestMemoryMaxBytes"].getInt64(INT64_MAX);
ResponseQueueCount = server["ResponseQueueCount"].getInt32(0);
if (ResponseQueueCount <= 0) {
ResponseQueueCount = ServerThreadCount / 10;
Expand Down

0 comments on commit aa63830

Please sign in to comment.