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 976 matching lines @@
 }
 
 void ThreadState::flushHeapDoesNotContainCacheIfNeeded() {
   if (m_shouldFlushHeapDoesNotContainCache) {
     m_heap->flushHeapDoesNotContainCache();
     m_shouldFlushHeapDoesNotContainCache = false;
   }
 }
 
 void ThreadState::makeConsistentForGC() {
+  if (isMainThread()) {

[haraken, 2016/12/02 12:43:20]

Maybe can we move this code into checkIfCompacting()? See my comment on
Visitor::create in this file.

[sof, 2016/12/04 14:55:38]

Moved the size sampling into the heap compaction code,
HeapCompact::updateHeapResidency() which is called while checking if compaction
should go ahead.

+    // For the main thread, report heap + freelist residency to the
+    // heap compactor. It uses the data to determine if compactions
+    // is now worthwhile for one or more of the sub heaps/arenas it can
+    // compact.
+    size_t heapSize = 0;
+    size_t freeSize = 0;

[haraken, 2016/12/02 12:43:21]

heapSize => totalArenaSize
freeSize => totalFreeListSize

[sof, 2016/12/04 14:55:38]

Done.

+    using Residency = std::pair<size_t, size_t>;
+    Vector<Residency> residencies;
+    NormalPageArena* arena;
+    for (int i = BlinkGC::Vector1ArenaIndex; i <= BlinkGC::HashTableArenaIndex;
+         ++i) {

[haraken, 2016/12/02 12:43:20]

Add an assertion somewhere to check that all backing store arenas are in
[Vector1ArenaIndex, HashTableArenaIndex]. Someone might change the enum order in
the future.

[sof, 2016/12/04 14:55:38]

Added (in HeapCompact::updateHeapResidency())

+      arena = static_cast<NormalPageArena*>(m_arenas[i]);
+      heapSize += arena->arenaSize();
+      freeSize += arena->freeListSize();
+      residencies.append(Residency(arena->arenaSize(), arena->freeListSize()));
+    }
+    heap().compaction()->setHeapResidency(heapSize, freeSize, residencies);
+  }
   ASSERT(isInGC());
   TRACE_EVENT0("blink_gc", "ThreadState::makeConsistentForGC");
   for (int i = 0; i < BlinkGC::NumberOfArenas; ++i)
     m_arenas[i]->makeConsistentForGC();
 }
 
+void ThreadState::compact() {

[haraken, 2016/12/02 12:43:20]

Add a TODO that the heap compaction should probably be enabled only on idle GCs.

[sof, 2016/12/04 14:55:38]

checkIfCompacting() is who decides this; added a TODO there. Looking at run-time
behavior, we may have to schedule some extra idle GCs if compaction is overdue,
to make that work out & have compaction be effective.

+  if (!heap().compaction()->isCompacting())

[haraken, 2016/12/02 12:43:20]

When can this happen?

[sof, 2016/12/04 14:55:38]

Most of the time? :) compact() is called unconditionally from preSweep().

+    return;
+

[haraken, 2016/12/02 12:43:20]

Let's enter SweepForbiddenScope and ScriptForbiddenIfMainThreadScope.

[sof, 2016/12/04 14:55:38]

Makes good sense, like the other sweeping passes.

+  // 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.)
+  //
+  heap().compaction()->startCompacting(this);
+  // TODO: implement bail out wrt any overall deadline, not
+  // compacting heaps if the time budget has been exceeded.
+  static_cast<NormalPageArena*>(m_arenas[BlinkGC::HashTableArenaIndex])
+      ->sweepCompact();

[haraken, 2016/12/02 12:43:20]

Can we move BlinkGC::HashTableArenaIndex into the following loop?

[sof, 2016/12/04 14:55:38]

Done.

+  for (int i = BlinkGC::Vector1ArenaIndex; i <= BlinkGC::InlineVectorArenaIndex;
+       ++i)
+    static_cast<NormalPageArena*>(m_arenas[i])->sweepCompact();
+  heap().compaction()->finishedCompacting(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();
+
 #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);

[haraken, 2016/12/02 12:43:20]

Maybe can we use this visitor creation down to line 1734, so that we can set the
proper marking mode when we create the visitor?

This CL is introducing a couple of setXXX() methods, which makes the oilpan
infrastructure more state-dependent (hard to maintain). For example,
checkIfCompacting changes the marking mode of the visitor by setMarkingMode.
makeConsistentForGC registers the heap state by setHeapResidency. I'd be happy
if we could remove such state-dependent code.

[sof, 2016/12/04 14:55:38]

I've teased apart the steps a bit to allow that, but it required making the
locking out of GCs explicit via (yet another) scoping object. Perhaps that's
slightly better overall, I don't see a clear winner between the two approaches.

 
   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;
 
+  heap().compaction()->checkIfCompacting(&heap(), visitor.get(), gcType,
+                                         reason);
+
   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 127 matching lines @@
     collectGarbage(BlinkGC::NoHeapPointersOnStack, BlinkGC::GCWithSweep,
                    BlinkGC::ForcedGC);
     size_t liveObjects = heap().heapStats().markedObjectSize();
     if (liveObjects == previousLiveObjects)
       break;
     previousLiveObjects = liveObjects;
   }
 }
 
 }  // namespace blink