Oilpan: Schedule a conservative GC when page navigations are happening frequently

Oilpan: Schedule a conservative GC when page navigations are happening frequently

A frame retains a lot of memory in the V8 side. For example, one v8::Context retained
by the frame has >200 KB. V8's GC cannot collect the memory until Oilpan's GC collects
the frame. So this CL forces a conservative GC if no Oilpan's GC has been observed in
the past 5 frame navigations.

This CL improves the performance of iframe-append-child as follows:

ToT without Oilpan: 177 runs/sec
ToT with Oilpan: 142 runs/sec
ToT + this CL with Oilpan: 168 runs/sec

This CL also reduces V8's peak memory usage as follows:

ToT without Oilpan: 34 MB
ToT with Oilpan: 110 MB
ToT + this CL with Oilpan: 50 MB

(Note: iframe-append-child uses little memory in the Blink side.
allocatedObjectSize + PartitionAlloc <= 4 MB.)

BUG=

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

[haraken, 2015-09-19 05:57:16]

haraken@chromium.org changed reviewers:
+ keishi@chromium.org, oilpan-reviews@chromium.org, sigbjornf@opera.se

[haraken, 2015-09-19 05:57:16]

PTAL

[keishi, 2015-09-19 06:12:15]

LGTM

I think this makes sense. It's a type of emergency page navigation GC thats
guaranteed to free a certain number of V8 objects.

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

https://codereview.chromium.org/1353283002/diff/1/Source/platform/heap/Thread...
Source/platform/heap/ThreadState.cpp:1411: m_pageNavigationCount = 0;
nit: I'm not sure if this belongs here because it isn't related to heapAges.

[haraken, 2015-09-19 08:17:14]

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

https://codereview.chromium.org/1353283002/diff/1/Source/platform/heap/Thread...
Source/platform/heap/ThreadState.cpp:1411: m_pageNavigationCount = 0;
On 2015/09/19 06:12:15, keishi wrote:
> nit: I'm not sure if this belongs here because it isn't related to heapAges.

Moved this to preGC.

[haraken, 2015-09-19 08:17:25]

The CQ bit was checked by haraken@chromium.org

[haraken, 2015-09-19 08:17:25]

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

[commit-bot: I haz the power, 2015-09-19 08:17:31]

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

[sof, 2015-09-19 08:21:22]

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

[sof, 2015-09-19 08:21:23]

lgtm to try.

[commit-bot: I haz the power, 2015-09-19 10:04:49]

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

[sof, 2015-09-21 07:15:00]

This one causes a conservative GC for every 5th child frame being loaded.

For a unit test like WebFrameTest.MaxFramesDetach, that extra work causes it to
timeout & fail.

[sof, 2015-09-21 07:43:50]

On 2015/09/21 07:15:00, sof wrote:
> This one causes a conservative GC for every 5th child frame being loaded.
> 
> For a unit test like WebFrameTest.MaxFramesDetach, that extra work causes it
to
> timeout & fail.

Unless this mechanism is restricted to "main frames" or otherwise curtails, this
change is problematic. i.e., it will put conservative GCs on the code path upon
loading a page containing a number of iframes (5 isn't that unusual), which is
preferably avoided.

Could you take a different approach for this perf test and consider doing
something upon removing iframes from the tree instead?

[haraken, 2015-09-21 09:00:42]

On 2015/09/21 07:43:50, sof wrote:
> On 2015/09/21 07:15:00, sof wrote:
> > This one causes a conservative GC for every 5th child frame being loaded.
> > 
> > For a unit test like WebFrameTest.MaxFramesDetach, that extra work causes it
> to
> > timeout & fail.
> 
> Unless this mechanism is restricted to "main frames" or otherwise curtails,
this
> change is problematic. i.e., it will put conservative GCs on the code path
upon
> loading a page containing a number of iframes (5 isn't that unusual), which is
> preferably avoided.
> 
> Could you take a different approach for this perf test and consider doing
> something upon removing iframes from the tree instead?

Thanks for pointing it out. Let me take a look.

[Ken Russell (switch to Gerrit), 2015-09-21 22:17:55]

A revert of this CL (patchset #2 id:20001) has been created in
https://codereview.chromium.org/1351943004/ by kbr@chromium.org.

The reason for reverting is: I believe this caused random crashes in the WebGL
conformance tests on all platforms as described in http://crbug.com/534524 .
.