Skip to content
Permalink
Browse files
Heap::collect shouldn't be responsible for sweeping
https://bugs.webkit.org/show_bug.cgi?id=126556

Reviewed by Geoffrey Garen.

Sweeping happens at an awkward time during collection due to the fact that destructors can
cause arbitrary reentry into the VM. This patch separates collecting and sweeping, and delays
sweeping until after collection has completely finished.

* heap/Heap.cpp:
(JSC::Heap::collectAllGarbage):
(JSC::Heap::collect):
(JSC::Heap::collectIfNecessaryOrDefer):
* heap/Heap.h:
* heap/MarkedSpace.cpp:
(JSC::MarkedSpace::sweep):
* runtime/GCActivityCallback.cpp:
(JSC::DefaultGCActivityCallback::doWork):


Canonical link: https://commits.webkit.org/144478@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@161429 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
Mark Hahnenberg committed Jan 7, 2014
1 parent bc0fa18 commit f8e1a478dc2eb9d7c3438b96152f53cebff6a9e9
Showing 5 changed files with 34 additions and 15 deletions.
@@ -1,3 +1,24 @@
2014-01-06 Mark Hahnenberg <mhahnenberg@apple.com>

Heap::collect shouldn't be responsible for sweeping
https://bugs.webkit.org/show_bug.cgi?id=126556

Reviewed by Geoffrey Garen.

Sweeping happens at an awkward time during collection due to the fact that destructors can
cause arbitrary reentry into the VM. This patch separates collecting and sweeping, and delays
sweeping until after collection has completely finished.

* heap/Heap.cpp:
(JSC::Heap::collectAllGarbage):
(JSC::Heap::collect):
(JSC::Heap::collectIfNecessaryOrDefer):
* heap/Heap.h:
* heap/MarkedSpace.cpp:
(JSC::MarkedSpace::sweep):
* runtime/GCActivityCallback.cpp:
(JSC::DefaultGCActivityCallback::doWork):

2014-01-07 Mark Rowe <mrowe@apple.com>

<https://webkit.org/b/126567> Remove the legacy WebKit availability macros
@@ -727,22 +727,26 @@ void Heap::collectAllGarbage()
{
if (!m_isSafeToCollect)
return;


collect();

SamplingRegion samplingRegion("Garbage Collection: Sweeping");
DelayedReleaseScope delayedReleaseScope(m_objectSpace);
collect(DoSweep);
m_objectSpace.sweep();
m_objectSpace.shrink();
}

static double minute = 60.0;

void Heap::collect(SweepToggle sweepToggle)
void Heap::collect()
{
#if ENABLE(ALLOCATION_LOGGING)
dataLogF("JSC GC starting collection.\n");
#endif

double before = 0;
if (Options::logGC()) {
dataLog("[GC", sweepToggle == DoSweep ? " (eager sweep)" : "", ": ");
dataLog("[GC: ");
before = currentTimeMS();
}

@@ -815,13 +819,6 @@ void Heap::collect(SweepToggle sweepToggle)
m_vm->clearSourceProviderCaches();
}

if (sweepToggle == DoSweep) {
SamplingRegion samplingRegion("Garbage Collection: Sweeping");
GCPHASE(Sweeping);
m_objectSpace.sweep();
m_objectSpace.shrink();
}

m_sweeper->startSweeping(m_blockSnapshot);
m_bytesAbandoned = 0;

@@ -880,7 +877,7 @@ bool Heap::collectIfNecessaryOrDefer()
if (!shouldCollect())
return false;

collect(DoNotSweep);
collect();
return true;
}

@@ -139,9 +139,8 @@ namespace JSC {
bool isSafeToCollect() const { return m_isSafeToCollect; }

JS_EXPORT_PRIVATE void collectAllGarbage();
enum SweepToggle { DoNotSweep, DoSweep };
bool shouldCollect();
void collect(SweepToggle);
void collect();
bool collectIfNecessaryOrDefer(); // Returns true if it did collect.

void reportExtraMemoryCost(size_t cost);
@@ -120,6 +120,8 @@ void MarkedSpace::lastChanceToFinalize()

void MarkedSpace::sweep()
{
if (Options::logGC())
dataLog("Eagerly sweeping...");
m_heap->sweeper()->willFinishSweeping();
forEachBlock<Sweep>();
}
@@ -95,7 +95,7 @@ void DefaultGCActivityCallback::doWork()
return;
}
#endif
heap->collect(Heap::DoNotSweep);
heap->collect();
}

#if USE(CF)

0 comments on commit f8e1a47

Please sign in to comment.