Skip to content

Commit

Permalink
Only quarantine garbage-collected objects
Browse files Browse the repository at this point in the history
Summary:
Testing with EagerGCProbability=1 is *really* slow, and lots of tests
time out. The biggest cost is quarantine; after each gc cycle, we turn
all free objects into Holes, to maximise bug-detection. This makes
the heap grow very rapidly, which makes heap-scanning very expensive.

This changes the collector to only quarantine gc-collected objects,
instead of all free objects. Gc-collected objects would have been
leaked anyway, so this limits heap growth to no worse than without gc.

Also, don't trigger "eager" gc cycles on free operations. Only do it
for allocations.

Reviewed By: ricklavoie

Differential Revision: D2765347

fb-gh-sync-id: 77554d7d6542d63d90b1c1387fb971ad6e1b3aee
  • Loading branch information
edwinsmith authored and hhvm-bot committed Dec 16, 2015
1 parent 606b1f2 commit bbbe77d
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 10 deletions.
7 changes: 5 additions & 2 deletions hphp/runtime/base/heap-collect.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -564,6 +564,9 @@ void Marker::sweep() {
ambig.count, ambig.bytes,
freed.count, freed.bytes);
// once we're done iterating the heap, it's safe to free unreachable objects.
if (debug && RuntimeOption::EvalEagerGCProbability > 0) {
mm.beginQuarantine();
}
for (auto h : reaped) {
if (h->kind() == HK::Apc) {
// frees localCache, delists, decref apc-array, free array
Expand All @@ -581,8 +584,8 @@ void Marker::sweep() {
mm.objFree(h, h->size());
}
}
if (RuntimeOption::EvalEagerGCProbability > 0) {
mm.quarantine();
if (debug && RuntimeOption::EvalEagerGCProbability > 0) {
mm.endQuarantine();
}
}
}
Expand Down
5 changes: 0 additions & 5 deletions hphp/runtime/base/memory-manager-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -268,8 +268,6 @@ inline void MemoryManager::freeSmallIndex(void* ptr, size_t index,
return freeBigSize(ptr, bytes);
}

if (debug) eagerGCCheck();

FTRACE(3, "freeSmallSize({}, {}), freelist {}\n", ptr, bytes, index);

m_freelists[index].push(ptr, bytes);
Expand All @@ -287,8 +285,6 @@ inline void MemoryManager::freeSmallSize(void* ptr, uint32_t bytes) {
return freeBigSize(ptr, bytes);
}

if (debug) eagerGCCheck();

auto const i = smallSize2Index(bytes);
FTRACE(3, "freeSmallSize({}, {}), freelist {}\n", ptr, bytes, i);

Expand All @@ -301,7 +297,6 @@ inline void MemoryManager::freeSmallSize(void* ptr, uint32_t bytes) {

ALWAYS_INLINE
void MemoryManager::freeBigSize(void* vp, size_t bytes) {
if (debug) eagerGCCheck();
m_stats.usage -= bytes;
// Since we account for these direct allocations in our usage and adjust for
// them on allocation, we also need to adjust for them negatively on free.
Expand Down
9 changes: 7 additions & 2 deletions hphp/runtime/base/memory-manager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -683,15 +683,20 @@ void MemoryManager::initFree() {
m_needInitFree = false;
}

// turn free blocks into holes, leave freelists empty.
void MemoryManager::quarantine() {
void MemoryManager::beginQuarantine() {
std::swap(m_freelists, m_quarantine);
}

// turn free blocks into holes, restore original freelists
void MemoryManager::endQuarantine() {
for (auto i = 0; i < kNumSmallSizes; i++) {
auto size = smallIndex2Size(i);
while (auto n = m_freelists[i].maybePop()) {
memset(n, 0x8a, size);
static_cast<FreeNode*>(n)->hdr.init(HeaderKind::Hole, size);
}
}
std::swap(m_freelists, m_quarantine);
}

// test iterating objects in slabs
Expand Down
11 changes: 10 additions & 1 deletion hphp/runtime/base/memory-manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -917,7 +917,13 @@ struct MemoryManager {
* Run the experimental collector.
*/
void collect(const char* phase);
void quarantine(); // turn free objects into holes

/*
* beginQuarantine() swaps out the normal freelists. endQuarantine()
* fills everything freed with holes, then restores the original freelists.
*/
void beginQuarantine();
void endQuarantine();

/*
* Run an integrity check on the heap
Expand Down Expand Up @@ -1103,6 +1109,9 @@ struct MemoryManager {
static size_t s_cactiveLimitCeiling;
bool m_enableStatsSync;
#endif

// freelists to use when quarantine is active
std::array<FreeList,kNumSmallSizes> m_quarantine;
};

//////////////////////////////////////////////////////////////////////
Expand Down

0 comments on commit bbbe77d

Please sign in to comment.