Remove a thread-local termination GC

Remove a thread-local termination GC

Now that we've shipped the per-thread heap, a thread-local termination GC
should not be needed. We can just use a normal GC to clean up the thread.

BUG=671856
Review-Url: https://codereview.chromium.org/2682933003
Cr-Commit-Position: refs/heads/master@{#449067}
Committed: https://chromium.googlesource.com/chromium/src/+/89d59827bf8706769513e1b6b0ad1a113bbfa65d

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

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

[haraken, 2017-02-08 09:31:15]

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

[haraken, 2017-02-08 09:31:16]

PTAL

I think this change is now safe if I'm not missing something.

[commit-bot: I haz the power, 2017-02-08 09:31:30]

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:15:29]

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

[commit-bot: I haz the power, 2017-02-08 11:15:30]

Dry run: This issue passed the CQ dry run.

[sof, 2017-02-08 11:49:47]

Not quite sure not having a simplified termination GC entrypoint is worth it.

https://codereview.chromium.org/2682933003/diff/1/third_party/WebKit/Source/p...
File third_party/WebKit/Source/platform/heap/ThreadState.cpp (right):

https://codereview.chromium.org/2682933003/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/heap/ThreadState.cpp:233: // Set the
terminate flag on all heap pages of this thread. This is used to
Could you make this comment up-to-date at the same time?

https://codereview.chromium.org/2682933003/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/heap/ThreadState.cpp:247:
collectGarbage(BlinkGC::NoHeapPointersOnStack, BlinkGC::GCWithSweep,
We're unnecessarily locking CTP access by doing this, and risk compacting --
what about the extra heap stat accounting done?

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

https://codereview.chromium.org/2682933003/diff/1/third_party/WebKit/Source/p...
File third_party/WebKit/Source/platform/heap/ThreadState.cpp (right):

https://codereview.chromium.org/2682933003/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/heap/ThreadState.cpp:233: // Set the
terminate flag on all heap pages of this thread. This is used to
On 2017/02/08 11:49:47, sof wrote:
> Could you make this comment up-to-date at the same time?

Maybe can I remove prepareForThreadStateTermination() in a follow-up after I've
confirmed this CL works on ToT? Added a TODO.

https://codereview.chromium.org/2682933003/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/heap/ThreadState.cpp:247:
collectGarbage(BlinkGC::NoHeapPointersOnStack, BlinkGC::GCWithSweep,
On 2017/02/08 11:49:47, sof wrote:
> We're unnecessarily locking CTP access by doing this, and risk compacting --
> what about the extra heap stat accounting done?

Added BlinkGC::ThreadTerminationGC. This prevents the GC from triggering a heap
compaction. See HeapCompact::shouldCompact().

I think we need to lock CTP though.

[sof, 2017-02-08 12:06:19]

ok, lgtm

(I assume you'll followup and remove collectGarbageForTerminatingThread() +
ThreadLocalMarking )

https://codereview.chromium.org/2682933003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/heap/ThreadState.cpp (right):

https://codereview.chromium.org/2682933003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/heap/ThreadState.cpp:236: // TODO(haraken):
Remove this. This is not needed once we remove a thread-
oh, i suppose that is now possible with orphaned pages gone.

[haraken, 2017-02-08 17:53:22]

The CQ bit was checked by haraken@chromium.org

[commit-bot: I haz the power, 2017-02-08 17:53:55]

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 19:50:46]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1486576402543340,
"parent_rev": "36f01a5102058f79be3548bf87903852c73c6523", "commit_rev":
"89d59827bf8706769513e1b6b0ad1a113bbfa65d"}

[commit-bot: I haz the power, 2017-02-08 19:51:23]

Description was changed from

==========
Remove a thread-local termination GC

Now that we've shipped the per-thread heap, a thread-local termination GC
should not be needed. We can just use a normal GC to clean up the thread.

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

to

==========
Remove a thread-local termination GC

Now that we've shipped the per-thread heap, a thread-local termination GC
should not be needed. We can just use a normal GC to clean up the thread.

BUG=671856

Review-Url: https://codereview.chromium.org/2682933003
Cr-Commit-Position: refs/heads/master@{#449067}
Committed:
https://chromium.googlesource.com/chromium/src/+/89d59827bf8706769513e1b6b0ad...
==========

[commit-bot: I haz the power, 2017-02-08 19:51:24]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/89d59827bf8706769513e1b6b0ad...