Oilpan: adjust GC policy under memory pressure (only.)

Oilpan: adjust GC policy under memory pressure (only.)

When notified of a v8 major GC, check if we're under memory pressure and
overdue an Oilpan GC & urgently. If so, force a conservative GC right
away.

This addresses dom-modify.html OOM troubles on win32, in particular. It
runs into the allocation pattern where the Oilpan allocator is able to
handle a tight loop just allocating Text nodes by bump allocating each
such node. Each Text node has a large external string allocation however,
which leads to a steep ramp-upin overall allocation amounts to the point
where it might OOM as no Oilpan GCs will run. v8 will however notice the
increasing memory pressure and schedule major GCs of its own.

Oilpan is notified after each v8 major GC, so use that opportunity to
check for memory pressure & force an Oilpan GC right away.

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

[sof, 2015-06-16 10:56:22]

sigbjornf@opera.com changed reviewers:
+ oilpan-reviews@chromium.org

[sof, 2015-06-16 10:56:23]

please take a look.

This is another take on https://codereview.chromium.org/1174123002/ (and
https://codereview.chromium.org/1178063004/ ), following the v8 gc perf
regression in crbug.com/499822.

It only allows conservative GCs to be taken if memory pressure is deemed to be
in effect following a v8 major GC, no other extra GCing will done otherwise.

[haraken, 2015-06-16 16:21:26]

haraken@chromium.org changed reviewers:
+ haraken@chromium.org

[haraken, 2015-06-16 16:21:27]

LGTM, thanks for taking care of this!

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

https://codereview.chromium.org/1190513006/diff/1/Source/platform/heap/Thread...
Source/platform/heap/ThreadState.cpp:833: }

Actually I don't fully understand an issue of calling scheduleGCIfNeeded() here.
What's wrong with it?

[sof, 2015-06-16 16:27:32]

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

https://codereview.chromium.org/1190513006/diff/1/Source/platform/heap/Thread...
Source/platform/heap/ThreadState.cpp:833: }
On 2015/06/16 16:21:27, haraken wrote:
> 
> Actually I don't fully understand an issue of calling scheduleGCIfNeeded()
here.
> What's wrong with it?

Oops, #if 0 - will undo that, ofc.

It triggering conservative GCs for other reasons appears to be behind the perf
regression, reporting longer v8 GC totals. I can reproduce that locally, but not
able to just with this CL (there is some noise though, so I'm not 100% sure this
will sail past the perf bots.)

[haraken, 2015-06-16 16:36:44]

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

https://codereview.chromium.org/1190513006/diff/1/Source/platform/heap/Thread...
Source/platform/heap/ThreadState.cpp:833: }
On 2015/06/16 16:27:31, sof wrote:
> On 2015/06/16 16:21:27, haraken wrote:
> > 
> > Actually I don't fully understand an issue of calling scheduleGCIfNeeded()
> here.
> > What's wrong with it?
> 
> Oops, #if 0 - will undo that, ofc.
> 
> It triggering conservative GCs for other reasons appears to be behind the perf
> regression, reporting longer v8 GC totals. I can reproduce that locally, but
not
> able to just with this CL (there is some noise though, so I'm not 100% sure
this
> will sail past the perf bots.)

I'm OK with landing this CL, but that sounds like a subtle optimization that
avoids benchmark regressions. If we observe a 400% increase after doing a major
GC, it would make sense to trigger a conservative GC.

This CL is a clear improvement, so we can revisit the issue in a follow up.

[sof, 2015-06-16 21:22:39]

On 2015/06/16 16:36:44, haraken wrote:
>
https://codereview.chromium.org/1190513006/diff/1/Source/platform/heap/Thread...
> File Source/platform/heap/ThreadState.cpp (right):
> 
>
https://codereview.chromium.org/1190513006/diff/1/Source/platform/heap/Thread...
> Source/platform/heap/ThreadState.cpp:833: }
> On 2015/06/16 16:27:31, sof wrote:
> > On 2015/06/16 16:21:27, haraken wrote:
> > > 
> > > Actually I don't fully understand an issue of calling scheduleGCIfNeeded()
> > here.
> > > What's wrong with it?
> > 
> > Oops, #if 0 - will undo that, ofc.
> > 
> > It triggering conservative GCs for other reasons appears to be behind the
perf
> > regression, reporting longer v8 GC totals. I can reproduce that locally, but
> not
> > able to just with this CL (there is some noise though, so I'm not 100% sure
> this
> > will sail past the perf bots.)
> 
> I'm OK with landing this CL, but that sounds like a subtle optimization that
> avoids benchmark regressions. If we observe a 400% increase after doing a
major
> GC, it would make sense to trigger a conservative GC.
> 
> This CL is a clear improvement, so we can revisit the issue in a follow up.

Yes, we need to put this OOM & dom-modify test behind us. I really wouldn't mind
by now :)

[sof, 2015-06-16 21:22:47]

The CQ bit was checked by sigbjornf@opera.com

[sof, 2015-06-16 21:22:47]

The patchset sent to the CQ was uploaded after l-g-t-m from haraken@chromium.org
Link to the patchset: https://codereview.chromium.org/1190513006/#ps20001
(title: "remove testing CPPery")

[commit-bot: I haz the power, 2015-06-16 21:23:02]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1190513006/20001

[commit-bot: I haz the power, 2015-06-17 01:47:48]

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

[commit-bot: I haz the power, 2015-06-17 01:47:50]

Try jobs failed on following builders:
  win_blink_rel on tryserver.blink (JOB_FAILED,
http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/66895)

[sof, 2015-06-17 05:34:38]

The CQ bit was checked by sigbjornf@opera.com

[commit-bot: I haz the power, 2015-06-17 05:34:46]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1190513006/20001

[sof, 2015-06-17 07:21:58]

The CQ bit was unchecked by sigbjornf@opera.com

[sof, 2015-06-17 07:22:42]

The CQ bit was checked by sigbjornf@opera.com

[commit-bot: I haz the power, 2015-06-17 07:22:50]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1190513006/20001

[commit-bot: I haz the power, 2015-06-17 08:59:16]

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