Simple BlinkGC heap compaction.

third_party/WebKit/Source/platform/heap/ThreadState.cpp

 /*
  * Copyright (C) 2013 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 20 matching lines @@
 #include "platform/heap/ThreadState.h"
 
 #include "base/trace_event/process_memory_dump.h"
 #include "platform/Histogram.h"
 #include "platform/RuntimeEnabledFeatures.h"
 #include "platform/ScriptForbiddenScope.h"
 #include "platform/heap/BlinkGCMemoryDumpProvider.h"
 #include "platform/heap/CallbackStack.h"
 #include "platform/heap/Handle.h"
 #include "platform/heap/Heap.h"
+#include "platform/heap/HeapCompact.h"
 #include "platform/heap/PagePool.h"
 #include "platform/heap/SafePoint.h"
 #include "platform/heap/Visitor.h"
 #include "platform/tracing/TraceEvent.h"
 #include "platform/tracing/web_memory_allocator_dump.h"
 #include "platform/tracing/web_process_memory_dump.h"
 #include "public/platform/Platform.h"
 #include "public/platform/WebScheduler.h"
 #include "public/platform/WebThread.h"
 #include "public/platform/WebTraceLocation.h"
@@ skipping 21 matching lines @@
 
 namespace blink {
 
 WTF::ThreadSpecific<ThreadState*>* ThreadState::s_threadSpecific = nullptr;
 uintptr_t ThreadState::s_mainThreadStackStart = 0;
 uintptr_t ThreadState::s_mainThreadUnderestimatedStackSize = 0;
 uint8_t ThreadState::s_mainThreadStateStorage[sizeof(ThreadState)];
 
 const size_t defaultAllocatedObjectSizeThreshold = 100 * 1024;
 
-const char* gcReasonString(BlinkGC::GCReason reason) {
+const char* ThreadState::gcReasonString(BlinkGC::GCReason reason) {
   switch (reason) {
     case BlinkGC::IdleGC:
       return "IdleGC";
     case BlinkGC::PreciseGC:
       return "PreciseGC";
     case BlinkGC::ConservativeGC:
       return "ConservativeGC";
     case BlinkGC::ForcedGC:
       return "ForcedGC";
     case BlinkGC::MemoryPressureGC:
@@ skipping 404 matching lines @@
   ScriptForbiddenIfMainThreadScope scriptForbiddenScope;
 
   // Disallow allocation during weak processing.
   // It would be technically safe to allow allocations, but it is unsafe
   // to mutate an object graph in a way in which a dead object gets
   // resurrected or mutate a HashTable (because HashTable's weak processing
   // assumes that the HashTable hasn't been mutated since the latest marking).
   // Due to the complexity, we just forbid allocations.
   NoAllocationScope noAllocationScope(this);
 
+  GCForbiddenScope gcForbiddenScope(this);

[haraken, 2016/12/09 07:25:56]

I'm curious why you need this. Isn't SweepForbiddenScope enough for preventing
GCs from getting triggered during the weak processing?

[sof, 2016/12/09 21:44:05]

See the Visitor constructor, rather than having it enter a GC forbidden scope
during its lifetime, we instead DCHECK() that we are when creating. Hence all
Visitor instances must be created while in such a scope - it seems best not to
try to overturn that setup here, and perhaps be more discriminating.

   std::unique_ptr<Visitor> visitor =
       Visitor::create(this, BlinkGC::ThreadLocalWeakProcessing);
 
   // Perform thread-specific weak processing.
   while (popAndInvokeThreadLocalWeakCallback(visitor.get())) {
   }
 
   m_threadLocalWeakCallbackStack->decommit();
 
   if (isMainThread()) {
@@ skipping 516 matching lines @@
   }
 }
 
 void ThreadState::makeConsistentForGC() {
   ASSERT(isInGC());
   TRACE_EVENT0("blink_gc", "ThreadState::makeConsistentForGC");
   for (int i = 0; i < BlinkGC::NumberOfArenas; ++i)
     m_arenas[i]->makeConsistentForGC();
 }
 
+void ThreadState::compact() {
+  if (!heap().compaction()->isCompacting())
+    return;
+
+  SweepForbiddenScope scope(this);
+  ScriptForbiddenIfMainThreadScope scriptForbiddenScope;

[haraken, 2016/12/09 07:25:56]

Also shall we add NoAllocationScope just in case?

[sof, 2016/12/09 21:44:05]

Done.

+
+  // Compaction is done eagerly and before the mutator threads get
+  // to run again. Doing it lazily is problematic, as the mutator's
+  // references to live objects could suddenly be invalidated by
+  // compaction of a page/heap. We do know all the references to
+  // the relocating objects just after marking, but won't later.
+  // (e.g., stack references could have been created, new objects
+  // created which refer to old collection objects, and so on.)
+
+  // Compact the hash table backing store arena first, it usually has
+  // higher fragmentation and is larger.
+  //
+  // TODO: implement bail out wrt any overall deadline, not compacting
+  // the remaining arenas if the time budget has been exceeded.
+  heap().compaction()->startThreadCompaction(this);
+  for (int i = BlinkGC::HashTableArenaIndex; i >= BlinkGC::Vector1ArenaIndex;
+       --i)
+    static_cast<NormalPageArena*>(m_arenas[i])->sweepAndCompact();
+  heap().compaction()->finishedThreadCompaction(this);
+}
+
 void ThreadState::makeConsistentForMutator() {
   ASSERT(isInGC());
   for (int i = 0; i < BlinkGC::NumberOfArenas; ++i)
     m_arenas[i]->makeConsistentForMutator();
 }
 
 void ThreadState::preGC() {
   if (RuntimeEnabledFeatures::traceWrappablesEnabled() && m_isolate &&
       m_performCleanup)
     m_performCleanup(m_isolate);
@@ skipping 63 matching lines @@
 
   // Allocation is allowed during the pre-finalizers and destructors.
   // However, they must not mutate an object graph in a way in which
   // a dead object gets resurrected.
   invokePreFinalizers();
 
   m_accumulatedSweepingTime = 0;
 
   eagerSweep();
 
+  compact();

[haraken, 2016/12/09 07:25:56]

Add a comment and mention why this must be put after pre-finalization and eager
sweeping.

[sof, 2016/12/09 21:44:05]

Done.

+
 #if defined(ADDRESS_SANITIZER)
   poisonAllHeaps();
 #endif
+
   if (previousGCState == EagerSweepScheduled) {
     // Eager sweeping should happen only in testing.
     completeSweep();
   } else {
     // The default behavior is lazy sweeping.
     scheduleIdleLazySweep();
   }
 }
 
 #if defined(ADDRESS_SANITIZER)
@@ skipping 528 matching lines @@
 
 void ThreadState::collectGarbage(BlinkGC::StackState stackState,
                                  BlinkGC::GCType gcType,
                                  BlinkGC::GCReason reason) {
   DCHECK_NE(gcType, BlinkGC::ThreadTerminationGC);
 
   // Nested collectGarbage() invocations aren't supported.
   RELEASE_ASSERT(!isGCForbidden());
   completeSweep();
 
-  std::unique_ptr<Visitor> visitor = Visitor::create(this, gcType);
+  GCForbiddenScope gcForbiddenScope(this);

[haraken, 2016/12/09 07:25:56]

I'm just curious but why did you add this? (I'm fine with adding it.)

[sof, 2016/12/09 21:44:05]

See the Visitor constructor comment, we had to introduce a GC forbidden lock
this early on to avoid some thread interference in the past. (I don't have the
bug references at hand).

This just preserves that; making changes in this area is high risk.

 
   SafePointScope safePointScope(stackState, this);
 
   // Resume all parked threads upon leaving this scope.
   ParkThreadsScope parkThreadsScope(this);
 
   // Try to park the other threads. If we're unable to, bail out of the GC.
   if (!parkThreadsScope.parkThreads())
     return;
 
+  BlinkGC::GCType visitorType = gcType;
+  if (heap().compaction()->shouldCompact(this, gcType, reason))
+    visitorType = heap().compaction()->initialize(this);

[haraken, 2016/12/09 07:25:56]

I'd prefer updating |gcType| instead of using |visitorType|, since the rest of
this function uses |gcType|.

[sof, 2016/12/09 21:44:05]

Notice that we do lose the with/without sweeping distinction for |gcType| when
creating the visitor via |visitorType|. We need to preserve that for later
|gcType| uses, which is why it is done this way.

[haraken, 2016/12/12 01:25:10]

Ah, got it. Would it be tidier to use a boolean flag (or enum) to specify if the
visitor should do the compaction or not?

  Visitor::create(this, gcType, ShouldCompactHeap);

+
+  std::unique_ptr<Visitor> visitor = Visitor::create(this, visitorType);
+
   ScriptForbiddenIfMainThreadScope scriptForbidden;
 
   TRACE_EVENT2("blink_gc,devtools.timeline", "BlinkGCMarking", "lazySweeping",
                gcType == BlinkGC::GCWithoutSweep, "gcReason",
                gcReasonString(reason));
   double startTime = WTF::currentTimeMS();
 
   if (gcType == BlinkGC::TakeSnapshot)
     BlinkGCMemoryDumpProvider::instance()->clearProcessDumpForCurrentGC();
 
@@ skipping 80 matching lines @@
   heap().postGC(gcType);
   heap().decommitCallbackStacks();
 }
 
 void ThreadState::collectGarbageForTerminatingThread() {
   {
     // A thread-specific termination GC must not allow other global GCs to go
     // ahead while it is running, hence the termination GC does not enter a
     // safepoint. VisitorScope will not enter also a safepoint scope for
     // ThreadTerminationGC.
+    GCForbiddenScope gcForbiddenScope(this);
     std::unique_ptr<Visitor> visitor =
         Visitor::create(this, BlinkGC::ThreadTerminationGC);
 
     ThreadState::NoAllocationScope noAllocationScope(this);
 
     heap().commitCallbackStacks();
     preGC();
 
     // 1. Trace the thread local persistent roots. For thread local GCs we
     // don't trace the stack (ie. no conservative scanning) since this is
@@ skipping 27 matching lines @@
     collectGarbage(BlinkGC::NoHeapPointersOnStack, BlinkGC::GCWithSweep,
                    BlinkGC::ForcedGC);
     size_t liveObjects = heap().heapStats().markedObjectSize();
     if (liveObjects == previousLiveObjects)
       break;
     previousLiveObjects = liveObjects;
   }
 }
 
 }  // namespace blink