Tune the memory pressure handler to perform a second GC immediately

Tune the memory pressure handler to perform a second GC immediately
after the first GC if time allows and there is memory to be freed.

BUG=chromium:618958
LOG=NO

Committed: https://crrev.com/29b695ef138f1fb23d179ffc3d7f0f97f6071b7e
Cr-Commit-Position: refs/heads/master@{#36894}

[ulan, 2016-06-10 11:06:10]

Description was changed from

==========
Tune the memory pressure handler to perform a second GC immediately
after the first GC if time allows and there is memory to be freed.

BUG=chromium:618958
LOG=NO
==========

to

==========
Tune the memory pressure handler to perform a second GC immediately
after the first GC if time allows and there is memory to be freed.

BUG=chromium:618958
LOG=NO
==========

[ulan, 2016-06-10 11:06:26]

ulan@chromium.org changed reviewers:
+ mlippautz@chromium.org

[ulan, 2016-06-10 11:06:27]

ptal, wdyt?

[Michael Lippautz, 2016-06-10 11:20:34]

Any chance we can deprecate LowMemoryNotification soonish? It's a bit confusing
to have 2 API calls solving a similar issue in different ways. Or maybe unify
their handling?

https://codereview.chromium.org/2057103002/diff/1/src/heap/heap.cc
File src/heap/heap.cc (right):

https://codereview.chromium.org/2057103002/diff/1/src/heap/heap.cc#newcode4471
src/heap/heap.cc:4471: const double kMaxMemoryPressurePauseMs = 100;
nit: If you can, please document why 100ms. (I was not part of all discussions
when this API landed.)

https://codereview.chromium.org/2057103002/diff/1/src/heap/heap.cc#newcode4476
src/heap/heap.cc:4476: new_space_.Shrink();
Why is the shrinking needed? This should be done through
GarbageCollectionEpilogue as ShouldReduceMemory() is true?

https://codereview.chromium.org/2057103002/diff/1/src/heap/heap.cc#newcode4494
src/heap/heap.cc:4494: if (FLAG_incremental_marking &&
incremental_marking()->IsStopped()) {
nit (not for now): At some point I think it would be good if maybe
StartIncrementalMarking and its variants would return a bool that returns
whether the start was successful. It's hard to remember that you have to check
the FLAG_ and IsStopped() when we want to start it.

[ulan, 2016-06-10 11:36:01]

https://codereview.chromium.org/2057103002/diff/1/src/heap/heap.cc
File src/heap/heap.cc (right):

https://codereview.chromium.org/2057103002/diff/1/src/heap/heap.cc#newcode4471
src/heap/heap.cc:4471: const double kMaxMemoryPressurePauseMs = 100;
On 2016/06/10 11:20:34, Michael Lippautz wrote:
> nit: If you can, please document why 100ms. (I was not part of all discussions
> when this API landed.)

Done.

https://codereview.chromium.org/2057103002/diff/1/src/heap/heap.cc#newcode4476
src/heap/heap.cc:4476: new_space_.Shrink();
On 2016/06/10 11:20:34, Michael Lippautz wrote:
> Why is the shrinking needed? This should be done through
> GarbageCollectionEpilogue as ShouldReduceMemory() is true?

Yes, thanks. Done.

https://codereview.chromium.org/2057103002/diff/1/src/heap/heap.cc#newcode4494
src/heap/heap.cc:4494: if (FLAG_incremental_marking &&
incremental_marking()->IsStopped()) {
On 2016/06/10 11:20:34, Michael Lippautz wrote:
> nit (not for now): At some point I think it would be good if maybe
> StartIncrementalMarking and its variants would return a bool that returns
> whether the start was successful. It's hard to remember that you have to check
> the FLAG_ and IsStopped() when we want to start it.

Acknowledged.

[Michael Lippautz, 2016-06-10 11:38:43]

lgtm

not sure why RAIL did not come to my mind when I saw the 100ms response time ;)

[ulan, 2016-06-10 11:43:05]

The CQ bit was checked by ulan@chromium.org

[commit-bot: I haz the power, 2016-06-10 11:43:12]

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

[commit-bot: I haz the power, 2016-06-10 12:01:41]

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

[commit-bot: I haz the power, 2016-06-10 12:01:42]

Try jobs failed on following builders:
  v8_linux_gcc_compile_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_gcc_compile_rel/bu...)

[ulan, 2016-06-10 12:50:32]

The CQ bit was checked by ulan@chromium.org

[ulan, 2016-06-10 12:50:32]

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

[commit-bot: I haz the power, 2016-06-10 12:50:34]

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

[commit-bot: I haz the power, 2016-06-10 13:17:16]

Description was changed from

==========
Tune the memory pressure handler to perform a second GC immediately
after the first GC if time allows and there is memory to be freed.

BUG=chromium:618958
LOG=NO
==========

to

==========
Tune the memory pressure handler to perform a second GC immediately
after the first GC if time allows and there is memory to be freed.

BUG=chromium:618958
LOG=NO
==========

[commit-bot: I haz the power, 2016-06-10 13:17:17]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-06-10 13:18:57]

Description was changed from

==========
Tune the memory pressure handler to perform a second GC immediately
after the first GC if time allows and there is memory to be freed.

BUG=chromium:618958
LOG=NO
==========

to

==========
Tune the memory pressure handler to perform a second GC immediately
after the first GC if time allows and there is memory to be freed.

BUG=chromium:618958
LOG=NO

Committed: https://crrev.com/29b695ef138f1fb23d179ffc3d7f0f97f6071b7e
Cr-Commit-Position: refs/heads/master@{#36894}
==========

[commit-bot: I haz the power, 2016-06-10 13:18:58]

Patchset 3 (id:??) landed as
https://crrev.com/29b695ef138f1fb23d179ffc3d7f0f97f6071b7e
Cr-Commit-Position: refs/heads/master@{#36894}