Oilpan: A precise GC should be prioritized than an idle GC

Oilpan: A precise GC should be prioritized than an idle GC

For now, this CL doesn't change any behavior since there is no opportunity
where precise GCs and idle GCs are scheduled at the same time.
(If ENABLE(IDLE_GC) is on, precise GCs are disabled. Otherwise, idle GCs are disabled.)

However, we're planning to mix precise GCs and idle GCs in the near future.
At that point, a precise GC needs to be more prioritized than an idle GC.
Specifically, this CL makes a change so that a precise GC is scheduled
when an idle GC is scheduled.

BUG=474470
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=200706

[haraken, 2015-08-18 02:38:15]

haraken@chromium.org changed reviewers:
+ keishi@chromium.org, oilpan-reviews@chromium.org, yutak@chromium.org

[haraken, 2015-08-18 02:39:53]

PTAL

https://codereview.chromium.org/1295713003/diff/1/Source/platform/heap/Thread...
File Source/platform/heap/ThreadState.cpp (right):

https://codereview.chromium.org/1295713003/diff/1/Source/platform/heap/Thread...
Source/platform/heap/ThreadState.cpp:654: return false;

yutak@: After landing this change, you just need to change this line to
judgeGCThreshold(1024 * 1024, xxx) to schedule a precise GC with a threshold
xxx.

[Yuta Kitamura, 2015-08-18 03:06:28]

https://codereview.chromium.org/1295713003/diff/1/Source/platform/heap/Thread...
File Source/platform/heap/ThreadState.cpp (left):

https://codereview.chromium.org/1295713003/diff/1/Source/platform/heap/Thread...
Source/platform/heap/ThreadState.cpp:654: return false;
Let me confirm my understanding of this removal.

* If an idle GC is already scheduled, it's fine to override it
  with a precise GC.
* Sweeping is never scheduled at this point.

Correct?

[haraken, 2015-08-18 03:45:23]

https://codereview.chromium.org/1295713003/diff/1/Source/platform/heap/Thread...
File Source/platform/heap/ThreadState.cpp (left):

https://codereview.chromium.org/1295713003/diff/1/Source/platform/heap/Thread...
Source/platform/heap/ThreadState.cpp:654: return false;
On 2015/08/18 03:06:28, Yuta Kitamura wrote:
> Let me confirm my understanding of this removal.
> 
> * If an idle GC is already scheduled, it's fine to override it
>   with a precise GC.

Yes, that is the intention of this CL.

> * Sweeping is never scheduled at this point.

Yes. scheduleGCIfNeeded() should not be called while the lazy sweeping is going
on. Even if it's called for some reason, scheduleGCIfNeeded() returns
immediately if isSweepingInProgress() is true (See line 704).

[Yuta Kitamura, 2015-08-18 04:41:56]

OK, LGTM.

[haraken, 2015-08-18 04:42:20]

The CQ bit was checked by haraken@chromium.org

[commit-bot: I haz the power, 2015-08-18 04:42:44]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1295713003/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1295713003/1

[sof, 2015-08-18 05:52:08]

sigbjornf@opera.com changed reviewers:
+ sigbjornf@opera.com

[sof, 2015-08-18 05:52:08]

lgtm.

IDLE_GC=0 isn't used by current build configs, so presumably this has been
tested with that set locally?

[haraken, 2015-08-18 06:10:14]

On 2015/08/18 05:52:08, sof wrote:
> lgtm.
> 
> IDLE_GC=0 isn't used by current build configs, so presumably this has been
> tested with that set locally?

Verified that it passes blink_heap_unittests with IDL_GC=0. It wouldn't cause an
issue since setting gcState(PreciseGCScheduled) multiple times is fine.

[commit-bot: I haz the power, 2015-08-18 06:35:44]

Committed patchset #1 (id:1) as
https://src.chromium.org/viewvc/blink?view=rev&revision=200706