[heap] More flag cleanup.

[heap] GC flag cleanup/restructuring.

* GC's flags are now proper flags and not int.
* Callback flags are not threaded through but only set once like gc flags
* Callers of methods that trigger GCs need to pass a reason when not using
  the default parameters.

Furthermore, each GC invocation can be passed the GC and GCCallback flags. We
usually override the currently set flags upon finishing a GC cylce, but are able
to restore the previously set if desired. This is useful for explicitely
triggered scavenges or external requests that interrupt the current behaviour.

BUG=

Committed: https://crrev.com/f4f3b431b9ce0778d926acf03c0d36dae5c0cba4
Cr-Commit-Position: refs/heads/master@{#30457}

[Michael Lippautz, 2015-08-25 14:25:13]

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

[Michael Lippautz, 2015-08-25 14:25:13]

More invasive than planned. The idea is that as soon as you call the GC with
non-default parameters you better have a reason why you do so.

The rest of the change is properly typing GC flags and making sure that the
both, the GCFlags and GCCallbackFlags, are properly set and reset.

[Hannes Payer (out of office), 2015-08-25 16:55:22]

lgtm

https://codereview.chromium.org/1314863003/diff/40001/src/heap/heap.cc
File src/heap/heap.cc (right):

https://codereview.chromium.org/1314863003/diff/40001/src/heap/heap.cc#newcod...
src/heap/heap.cc:825: Heap::kMakeHeapIterableMask |
Heap::kReduceMemoryFootprintMask,
Let's rename this instance to kAbortIncrementalMarkingMask. I think it is
clearer in this context.

[Michael Lippautz, 2015-08-25 17:13:49]

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

[Michael Lippautz, 2015-08-25 17:13:49]

+yang for debug/*

https://codereview.chromium.org/1314863003/diff/40001/src/heap/heap.cc
File src/heap/heap.cc (right):

https://codereview.chromium.org/1314863003/diff/40001/src/heap/heap.cc#newcod...
src/heap/heap.cc:825: Heap::kMakeHeapIterableMask |
Heap::kReduceMemoryFootprintMask,
On 2015/08/25 16:55:22, Hannes Payer wrote:
> Let's rename this instance to kAbortIncrementalMarkingMask. I think it is
> clearer in this context.

Done.

[Yang, 2015-08-26 10:20:20]

On 2015/08/25 17:13:49, Michael Lippautz wrote:
> +yang for debug/*
> 
> https://codereview.chromium.org/1314863003/diff/40001/src/heap/heap.cc
> File src/heap/heap.cc (right):
> 
>
https://codereview.chromium.org/1314863003/diff/40001/src/heap/heap.cc#newcod...
> src/heap/heap.cc:825: Heap::kMakeHeapIterableMask |
> Heap::kReduceMemoryFootprintMask,
> On 2015/08/25 16:55:22, Hannes Payer wrote:
> > Let's rename this instance to kAbortIncrementalMarkingMask. I think it is
> > clearer in this context.
> 
> Done.

lgtm

[Michael Lippautz, 2015-08-26 10:25:16]

The CQ bit was checked by mlippautz@chromium.org

[Michael Lippautz, 2015-08-26 10:25:17]

The patchset sent to the CQ was uploaded after l-g-t-m from hpayer@chromium.org
Link to the patchset: https://codereview.chromium.org/1314863003/#ps60001
(title: "addressed comment + rebase")

[commit-bot: I haz the power, 2015-08-26 10:25:26]

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

[commit-bot: I haz the power, 2015-08-26 10:35:14]

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

[commit-bot: I haz the power, 2015-08-26 10:35:15]

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

[Michael Lippautz, 2015-08-26 11:51:25]

Hannes PTAL;

It's more invasive than initially planned. The current CL addresses all the
issues we had with flags though.

You can override/save existing flags while calling the GC for any space now.
Incremental marking always sets the flags.

The reason I went for the scope is that the API function invoking a "minor" GC
wants to perform a scavenge with forcing the callback flags.

[Michael Lippautz, 2015-08-26 12:45:39]

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

[commit-bot: I haz the power, 2015-08-26 12:45:45]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1314863003/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1314863003/120001

[commit-bot: I haz the power, 2015-08-26 13:12:03]

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

[commit-bot: I haz the power, 2015-08-26 13:12:03]

Dry run: This issue passed the CQ dry run.

[Hannes Payer (out of office), 2015-08-26 21:57:28]

I am a bit worried about the complexity of the current approach.
In principle, we could have to pairs of flags in heap
scavenge_gc_callback_flags and scavenge_gc_flags (set when scavenge is
performed)
mark_compact_gc_callback_flags and mark_compact_gc_flags (set when incremental
marking is started and are kept until incremental marking/full gc
finishes/aborts or when full GC is performed).

Why would you be in favor of the current approach?

[Hannes Payer (out of office), 2015-08-27 12:41:14]

I am not a big fan of the involved complexity, but the alternatives are also not
great. So LGTM.
Please describe in detail what the semantics are and what may happen in
different situation involving incremental marking, scavenges, full gc...

https://codereview.chromium.org/1314863003/diff/120001/src/heap/heap.h
File src/heap/heap.h (right):

https://codereview.chromium.org/1314863003/diff/120001/src/heap/heap.h#newcod...
src/heap/heap.h:1357: // collect more garbage.
Explain in detail here how the flags work.

https://codereview.chromium.org/1314863003/diff/120001/src/heap/heap.h#newcod...
src/heap/heap.h:1364: inline bool CollectGarbageNewSpace(const char* gc_reason =
nullptr);
Also mention here what impact the flags have.

[Michael Lippautz, 2015-08-31 07:32:22]

The CQ bit was checked by mlippautz@chromium.org

[Michael Lippautz, 2015-08-31 07:32:22]

The patchset sent to the CQ was uploaded after l-g-t-m from
yangguo@chromium.org, hpayer@chromium.org
Link to the patchset: https://codereview.chromium.org/1314863003/#ps140001
(title: "Added more comments")

[commit-bot: I haz the power, 2015-08-31 07:32:35]

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

[commit-bot: I haz the power, 2015-08-31 07:58:47]

Committed patchset #8 (id:140001)

[commit-bot: I haz the power, 2015-08-31 07:59:07]

Patchset 8 (id:??) landed as
https://crrev.com/f4f3b431b9ce0778d926acf03c0d36dae5c0cba4
Cr-Commit-Position: refs/heads/master@{#30457}

[Michael Hablich, 2015-08-31 10:23:07]

A revert of this CL (patchset #8 id:140001) has been created in
https://codereview.chromium.org/1303393004/ by hablich@chromium.org.

The reason for reverting is: Breaks
http://build.chromium.org/p/client.v8/builders/V8%20Arm%20-%20debug%20-%202/b....