Remove orphaned pages from Oilpan

Remove orphaned pages from Oilpan

Now that we have shipped the per-thread heap, orphaned pages are not needed.

orphaned pages was needed because 1) a thread termination GC traces only objects
in the thread's heap and 2) there are cross-thread Member pointers. This means
that after finishing the thread termination GC, it's possible that some Member
pointers are still pointing to the thread's heap. Thus we couldn't immediately
release the thread's heap. Hence we had to mark the pages in the heap as orphaned
until a next GC is triggered.

In the per-thread heap world, cross-thread pointers must be CrossThreadPersistents.
Also we have a logic to clear CrossThreadPersistents pointing to the terminating
thread's heap before the thread terminates. This means that it's guaranteed that
there is no cross-thread pointer pointing to the terminating thread's heap
after the thread terminates. Then orphaned pages are not needed.

BUG=671856
Review-Url: https://codereview.chromium.org/2684633004
Cr-Commit-Position: refs/heads/master@{#449225}
Committed: https://chromium.googlesource.com/chromium/src/+/665243239207d7a0e1e73a4179c55f083c5493ca

[haraken, 2017-02-08 11:07:02]

The CQ bit was checked by haraken@chromium.org to run a CQ dry run

[haraken, 2017-02-08 11:07:09]

haraken@chromium.org changed reviewers:
+ sigbjornf@opera.com

[haraken, 2017-02-08 11:07:10]

PTAL

[commit-bot: I haz the power, 2017-02-08 11:07:23]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-08 11:37:52]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-08 11:37:53]

Dry run: Try jobs failed on following builders:
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)

[haraken, 2017-02-08 11:41:52]

The CQ bit was checked by haraken@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-08 11:42:09]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[sof, 2017-02-08 11:58:08]

lgtm

https://codereview.chromium.org/2684633004/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/heap/PagePool.h (right):

https://codereview.chromium.org/2684633004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/heap/PagePool.h:16: template <typename
DataType>
we can drop the parameterization of PagePool<> now.

[haraken, 2017-02-08 11:59:54]

https://codereview.chromium.org/2684633004/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/heap/PagePool.h (right):

https://codereview.chromium.org/2684633004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/heap/PagePool.h:16: template <typename
DataType>
On 2017/02/08 11:58:08, sof wrote:
> we can drop the parameterization of PagePool<> now.

Good point. Let me do that in a follow-up.

[haraken, 2017-02-08 12:00:00]

The CQ bit was unchecked by haraken@chromium.org

[haraken, 2017-02-08 12:00:04]

The CQ bit was checked by haraken@chromium.org

[commit-bot: I haz the power, 2017-02-08 12:00:15]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-08 13:38:56]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-08 13:38:59]

Failed to apply patch for
third_party/WebKit/Source/platform/heap/ThreadState.cpp:
While running git apply --index -p1;
  error: patch failed:
third_party/WebKit/Source/platform/heap/ThreadState.cpp:258
  error: third_party/WebKit/Source/platform/heap/ThreadState.cpp: patch does not
apply

Patch:       third_party/WebKit/Source/platform/heap/ThreadState.cpp
Index: third_party/WebKit/Source/platform/heap/ThreadState.cpp
diff --git a/third_party/WebKit/Source/platform/heap/ThreadState.cpp
b/third_party/WebKit/Source/platform/heap/ThreadState.cpp
index
a656dfecef2e02a9c2983e0263dc42c4602ed4d9..ad5a8fedd237291941a8934175e123986d0fdf3a
100644
--- a/third_party/WebKit/Source/platform/heap/ThreadState.cpp
+++ b/third_party/WebKit/Source/platform/heap/ThreadState.cpp
@@ -208,15 +208,15 @@ void ThreadState::attachCurrentThread() {
   new ThreadState();
 }
 
-void ThreadState::cleanupPages() {
+void ThreadState::removeAllPages() {
   ASSERT(checkThread());
   for (int i = 0; i < BlinkGC::NumberOfArenas; ++i)
-    m_arenas[i]->cleanupPages();
+    m_arenas[i]->removeAllPages();
 }
 
 void ThreadState::runTerminationGC() {
   if (isMainThread()) {
-    cleanupPages();
+    removeAllPages();
     return;
   }
   ASSERT(checkThread());
@@ -258,9 +258,7 @@ void ThreadState::runTerminationGC() {
   ASSERT(m_orderedPreFinalizers.isEmpty());
   RELEASE_ASSERT(gcState() == NoGCScheduled);
 
-  // Add pages to the orphaned page pool to ensure any global GCs from this
-  // point on will not trace objects on this thread's arenas.
-  cleanupPages();
+  removeAllPages();
 }
 
 void ThreadState::cleanupMainThread() {
@@ -392,11 +390,6 @@ void ThreadState::pushThreadLocalWeakCallback(void* object,
 
 bool ThreadState::popAndInvokeThreadLocalWeakCallback(Visitor* visitor) {
   ASSERT(checkThread());
-  // For weak processing we should never reach orphaned pages since orphaned
-  // pages are not traced and thus objects on those pages are never be
-  // registered as objects on orphaned pages. We cannot assert this here since
-  // we might have an off-heap collection. We assert it in
-  // ThreadHeap::pushThreadLocalWeakCallback.
   if (CallbackStack::Item* item = m_threadLocalWeakCallbackStack->pop()) {
     item->call(visitor);
     return true;
@@ -1684,11 +1677,6 @@ void ThreadState::collectGarbage(BlinkGC::StackState
stackState,
       heap().globalWeakProcessing(visitor.get());
     }
 
-    // Now we can delete all orphaned pages because there are no dangling
-    // pointers to the orphaned pages.  (If we have such dangling pointers,
-    // we should have crashed during marking before getting here.)
-    heap().getOrphanedPagePool()->decommitOrphanedPages();
-
     double markingTimeInMilliseconds = WTF::currentTimeMS() - startTime;
     heap().heapStats().setEstimatedMarkingTimePerByte(
         totalObjectSize ? (markingTimeInMilliseconds / 1000 / totalObjectSize)

[haraken, 2017-02-09 01:32:37]

The CQ bit was checked by haraken@chromium.org

[haraken, 2017-02-09 01:32:37]

The patchset sent to the CQ was uploaded after l-g-t-m from sigbjornf@opera.com
Link to the patchset: https://codereview.chromium.org/2684633004/#ps40001
(title: "temp")

[commit-bot: I haz the power, 2017-02-09 01:33:26]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-09 06:21:42]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1486603957213150,
"parent_rev": "b611e84af4f6d181789aab819af189c3e6c71b9f", "commit_rev":
"665243239207d7a0e1e73a4179c55f083c5493ca"}

[commit-bot: I haz the power, 2017-02-09 06:22:15]

Description was changed from

==========
Remove orphaned pages from Oilpan

Now that we have shipped the per-thread heap, orphaned pages are not needed.

orphaned pages was needed because 1) a thread termination GC traces only objects
in the thread's heap and 2) there are cross-thread Member pointers. This means
that after finishing the thread termination GC, it's possible that some Member
pointers are still pointing to the thread's heap. Thus we couldn't immediately
release the thread's heap. Hence we had to mark the pages in the heap as
orphaned
until a next GC is triggered.

In the per-thread heap world, cross-thread pointers must be
CrossThreadPersistents.
Also we have a logic to clear CrossThreadPersistents pointing to the terminating
thread's heap before the thread terminates. This means that it's guaranteed that
there is no cross-thread pointer pointing to the terminating thread's heap
after the thread terminates. Then orphaned pages are not needed.

BUG=671856
==========

to

==========
Remove orphaned pages from Oilpan

Now that we have shipped the per-thread heap, orphaned pages are not needed.

orphaned pages was needed because 1) a thread termination GC traces only objects
in the thread's heap and 2) there are cross-thread Member pointers. This means
that after finishing the thread termination GC, it's possible that some Member
pointers are still pointing to the thread's heap. Thus we couldn't immediately
release the thread's heap. Hence we had to mark the pages in the heap as
orphaned
until a next GC is triggered.

In the per-thread heap world, cross-thread pointers must be
CrossThreadPersistents.
Also we have a logic to clear CrossThreadPersistents pointing to the terminating
thread's heap before the thread terminates. This means that it's guaranteed that
there is no cross-thread pointer pointing to the terminating thread's heap
after the thread terminates. Then orphaned pages are not needed.

BUG=671856

Review-Url: https://codereview.chromium.org/2684633004
Cr-Commit-Position: refs/heads/master@{#449225}
Committed:
https://chromium.googlesource.com/chromium/src/+/665243239207d7a0e1e73a4179c5...
==========

[commit-bot: I haz the power, 2017-02-09 06:22:16]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/665243239207d7a0e1e73a4179c5...