[heap] Add GC accounting to slow allocation and incremental marking job

[heap] Add GC accounting to slow allocation and incremental marking job

BUG=v8:6343
Review-Url: https://codereview.chromium.org/2861763002
Cr-Commit-Position: refs/heads/master@{#45073}
Committed: https://chromium.googlesource.com/v8/v8/+/8ab39ebcf90e6e838dd5ca2d6bbfdd8111841777

[Michael Lippautz, 2017-05-03 15:44:42]

Description was changed from

==========
[heap] Add GC accounting to slow allocation and incremental marking job

BUG=
==========

to

==========
[heap] Add GC accounting to slow allocation and incremental marking job

BUG=v8:6343
==========

[Michael Lippautz, 2017-05-03 15:44:43]

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

[Michael Lippautz, 2017-05-03 15:44:52]

ptal

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

https://codereview.chromium.org/2861763002/diff/1/src/heap/spaces.cc#newcode2903
src/heap/spaces.cc:2903: VMState<GC> state(heap()->isolate());
I think this is a pretty good estimate here that avoids polluting the fast path.
wdyt?

[Michael Lippautz, 2017-05-03 15:45:13]

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

[commit-bot: I haz the power, 2017-05-03 15:45:14]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Michael Lippautz, 2017-05-03 15:47:07]

Camillo: I was not sure whether we want to have the runtime stats too, wdyt?

[commit-bot: I haz the power, 2017-05-03 15:47:44]

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

[commit-bot: I haz the power, 2017-05-03 15:47:45]

Dry run: Try jobs failed on following builders:
  v8_linux64_avx2_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel_ng/buil...)
  v8_linux64_gyp_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_gyp_rel_ng/build...)
  v8_linux64_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_rel_ng/builds/25400)
  v8_linux64_verify_csa_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_verify_csa_rel_n...)
  v8_linux_arm_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm_rel_ng/builds/...)
  v8_linux_gcc_compile_rel on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_gcc_compile_rel/bu...)
  v8_linux_mips64el_compile_rel on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_mips64el_compile_r...)
  v8_linux_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_rel_ng/builds/25308)
  v8_mac_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_mac_rel_ng/builds/21923)

[Michael Lippautz, 2017-05-03 15:52:00]

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

[commit-bot: I haz the power, 2017-05-03 15:52:05]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Camillo Bruni, 2017-05-03 16:24:19]

On 2017/05/03 at 15:47:07, mlippautz wrote:
> Camillo: I was not sure whether we want to have the runtime stats too, wdyt?

I guess this would be nice. Do you remember which places we still miss out?
Let's land this CL as is and maybe have a a follow-up CL.

[Camillo Bruni, 2017-05-03 16:24:45]

On 2017/05/03 at 16:24:19, Camillo Bruni wrote:
> On 2017/05/03 at 15:47:07, mlippautz wrote:
> > Camillo: I was not sure whether we want to have the runtime stats too, wdyt?
> 
> I guess this would be nice. Do you remember which places we still miss out?
> Let's land this CL as is and maybe have a a follow-up CL.

ah now I see, yes fine :)

[Camillo Bruni, 2017-05-03 16:25:57]

LGTM with remarks.

https://codereview.chromium.org/2861763002/diff/20001/src/heap/incremental-ma...
File src/heap/incremental-marking-job.cc (right):

https://codereview.chromium.org/2861763002/diff/20001/src/heap/incremental-ma...
src/heap/incremental-marking-job.cc:47: RuntimeCallTimerScope(isolate(),
&RuntimeCallStats::GC);
we could even add a separate timer? GC_IncremenetalMarking (see counters.h)

https://codereview.chromium.org/2861763002/diff/20001/src/heap/spaces.cc
File src/heap/spaces.cc (right):

https://codereview.chromium.org/2861763002/diff/20001/src/heap/spaces.cc#newc...
src/heap/spaces.cc:2905: RuntimeCallTimerScope(heap()->isolate(),
&RuntimeCallStats::GC);
same here?

[commit-bot: I haz the power, 2017-05-03 16:28:40]

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

[commit-bot: I haz the power, 2017-05-03 16:28:41]

Dry run: This issue passed the CQ dry run.

[Michael Lippautz, 2017-05-03 16:32:00]

https://codereview.chromium.org/2861763002/diff/20001/src/heap/incremental-ma...
File src/heap/incremental-marking-job.cc (right):

https://codereview.chromium.org/2861763002/diff/20001/src/heap/incremental-ma...
src/heap/incremental-marking-job.cc:47: RuntimeCallTimerScope(isolate(),
&RuntimeCallStats::GC);
On 2017/05/03 16:25:56, Camillo Bruni wrote:
> we could even add a separate timer? GC_IncremenetalMarking (see counters.h)

GC_IncrementalMarkingJob (can do sweep and/or marking)

https://codereview.chromium.org/2861763002/diff/20001/src/heap/spaces.cc
File src/heap/spaces.cc (right):

https://codereview.chromium.org/2861763002/diff/20001/src/heap/spaces.cc#newc...
src/heap/spaces.cc:2905: RuntimeCallTimerScope(heap()->isolate(),
&RuntimeCallStats::GC);
On 2017/05/03 16:25:56, Camillo Bruni wrote:
> same here?

Used GC_SlowAllocateRaw as this one here can do sweeping and/or marking.

[Michael Lippautz, 2017-05-03 16:41:59]

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

[commit-bot: I haz the power, 2017-05-03 16:42:07]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-03 17:13:30]

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

[commit-bot: I haz the power, 2017-05-03 17:13:31]

Dry run: This issue passed the CQ dry run.

[Michael Lippautz, 2017-05-03 18:36:19]

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

[commit-bot: I haz the power, 2017-05-03 18:36:21]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[ulan, 2017-05-03 18:36:43]

lgtm

[commit-bot: I haz the power, 2017-05-03 18:39:51]

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

[commit-bot: I haz the power, 2017-05-03 18:39:51]

Dry run: Try jobs failed on following builders:
  v8_android_arm_compile_rel on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_android_arm_compile_rel/...)
  v8_linux64_asan_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_asan_rel_ng/buil...)
  v8_linux64_gyp_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_gyp_rel_ng/build...)
  v8_linux_dbg_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_dbg_ng/builds/25403)
  v8_linux_nodcheck_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_nodcheck_rel_ng/bu...)
  v8_linux_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_rel_ng/builds/25320)
  v8_linux_verify_csa_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_verify_csa_rel_ng/...)

[Michael Lippautz, 2017-05-03 19:02:18]

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

[commit-bot: I haz the power, 2017-05-03 19:02:21]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-03 19:33:26]

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

[commit-bot: I haz the power, 2017-05-03 19:33:27]

Dry run: This issue passed the CQ dry run.

[Hannes Payer (out of office), 2017-05-03 19:58:37]

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

[Hannes Payer (out of office), 2017-05-03 19:58:38]

lgtm

[Michael Lippautz, 2017-05-03 20:57:32]

The CQ bit was checked by mlippautz@chromium.org

[Michael Lippautz, 2017-05-03 20:57:33]

The patchset sent to the CQ was uploaded after l-g-t-m from cbruni@chromium.org,
ulan@chromium.org
Link to the patchset: https://codereview.chromium.org/2861763002/#ps80001
(title: "Fix typo")

[Michael Lippautz, 2017-05-03 20:57:33]

The CQ bit was unchecked by mlippautz@chromium.org

[Michael Lippautz, 2017-05-03 20:57:43]

The CQ bit was checked by mlippautz@chromium.org

[commit-bot: I haz the power, 2017-05-03 20:57:50]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-03 20:59:30]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1493845063145290,
"parent_rev": "644379eedc8d0442e92e7a7713dcfe490d5c214b", "commit_rev":
"8ab39ebcf90e6e838dd5ca2d6bbfdd8111841777"}

[commit-bot: I haz the power, 2017-05-03 20:59:38]

Description was changed from

==========
[heap] Add GC accounting to slow allocation and incremental marking job

BUG=v8:6343
==========

to

==========
[heap] Add GC accounting to slow allocation and incremental marking job

BUG=v8:6343

Review-Url: https://codereview.chromium.org/2861763002
Cr-Commit-Position: refs/heads/master@{#45073}
Committed:
https://chromium.googlesource.com/v8/v8/+/8ab39ebcf90e6e838dd5ca2d6bbfdd81118...
==========

[commit-bot: I haz the power, 2017-05-03 20:59:39]

Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/v8/v8/+/8ab39ebcf90e6e838dd5ca2d6bbfdd81118...