Add GC stats to GC timeline events

Add GC stats to GC timeline events

BUG=
R=rmacnak@google.com

Committed: https://github.com/dart-lang/sdk/commit/f07fd20304e97238ae619f6f69b8cd9bd9b15530

[Cutch, 2016-05-09 17:09:16]

Description was changed from

==========
Add GC stats to GC timeline events

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

to

==========
Add GC stats to GC timeline events

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

[Cutch, 2016-05-09 17:09:16]

johnmccutchan@google.com changed reviewers:
+ iposva@google.com, rmacnak@google.com

[rmacnak, 2016-05-09 19:32:17]

https://codereview.chromium.org/1961033003/diff/1/runtime/vm/heap.cc
File runtime/vm/heap.cc (right):

https://codereview.chromium.org/1961033003/diff/1/runtime/vm/heap.cc#newcode846
runtime/vm/heap.cc:846: "%" Pd "",
It's hard to figure out the units just from looking at the event. Either don't
round to kB or put a kB suffix on the string.

[Ivan Posva, 2016-05-09 19:39:58]

https://codereview.chromium.org/1961033003/diff/1/runtime/vm/heap.cc
File runtime/vm/heap.cc (right):

https://codereview.chromium.org/1961033003/diff/1/runtime/vm/heap.cc#newcode846
runtime/vm/heap.cc:846: "%" Pd "",
On 2016/05/09 19:32:17, rmacnak wrote:
> It's hard to figure out the units just from looking at the event. Either don't
> round to kB or put a kB suffix on the string.

I guess another option is to put the unit on the name such as "Before.New.Used
(kB)" which will give the units without polluting the value in case somebody
wants to parse it out.

https://codereview.chromium.org/1961033003/diff/1/runtime/vm/heap.cc#newcode893
runtime/vm/heap.cc:893: RoundWordsToKB(stats_.after_.old_.external_in_words));
I would also like to add the times_ values, but they change from new to old gen.
So a possibility would be to pass their description as a parameter or somehow
record the description in the stats object.

[Cutch, 2016-05-09 20:45:01]

https://codereview.chromium.org/1961033003/diff/1/runtime/vm/heap.cc
File runtime/vm/heap.cc (right):

https://codereview.chromium.org/1961033003/diff/1/runtime/vm/heap.cc#newcode846
runtime/vm/heap.cc:846: "%" Pd "",
On 2016/05/09 19:39:58, Ivan Posva wrote:
> On 2016/05/09 19:32:17, rmacnak wrote:
> > It's hard to figure out the units just from looking at the event. Either
don't
> > round to kB or put a kB suffix on the string.
> 
> I guess another option is to put the unit on the name such as "Before.New.Used
> (kB)" which will give the units without polluting the value in case somebody
> wants to parse it out.

Done.

https://codereview.chromium.org/1961033003/diff/1/runtime/vm/heap.cc#newcode893
runtime/vm/heap.cc:893: RoundWordsToKB(stats_.after_.old_.external_in_words));
On 2016/05/09 19:39:58, Ivan Posva wrote:
> I would also like to add the times_ values, but they change from new to old
gen.
> So a possibility would be to pass their description as a parameter or somehow
> record the description in the stats object.

Acknowledged.

[rmacnak, 2016-05-09 21:27:52]

lgtm

[Cutch, 2016-05-09 21:29:03]

Description was changed from

==========
Add GC stats to GC timeline events

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

to

==========
Add GC stats to GC timeline events

BUG=
R=rmacnak@google.com

Committed:
https://github.com/dart-lang/sdk/commit/f07fd20304e97238ae619f6f69b8cd9bd9b15530
==========

[Cutch, 2016-05-09 21:29:07]

Committed patchset #2 (id:20001) manually as
f07fd20304e97238ae619f6f69b8cd9bd9b15530 (presubmit successful).