Visual refresh of allocation profile page

Visual refresh of allocation profile page

- Redo layout of page.
- Show human readable strings for used, free, and external memory for each space.
- Track time of last forced GC/accumulator reset in isolate and display them in allocation profile page.
- Track allocation statistics on Class object itself.
- Make isolate's heap reporting use heap's PrintToJSON.
- Manually insert allocation profile table rows into dom (removes usage of template repeat).
- Add some timing information to allocation profile page update (to help diagnose slow downs).


Other changes:
- Upgrade to Polymer 0.11-dev (finally ends erroneous console spam).
- Replace enteredView/leftView with attached/detached (polymer change).
- Add <link> tag to polymer.html for each element (polymer change).
- Move <script> tag outside of element (uniformity, clarity).
- Remove unused collapsible-content element.

R=koda@google.com

Committed: https://code.google.com/p/dart/source/detail?r=37456

[koda, 2014-06-17 21:04:07]

The demo looked like a great improvement!

https://codereview.chromium.org/342513004/diff/40001/runtime/bin/vmservice/cl...
File runtime/bin/vmservice/client/lib/src/app/view_model.dart (right):

https://codereview.chromium.org/342513004/diff/40001/runtime/bin/vmservice/cl...
runtime/bin/vmservice/client/lib/src/app/view_model.dart:137: //print('$label
took ${sw.elapsedMicroseconds} us.');
Uncomment or remove.

https://codereview.chromium.org/342513004/diff/40001/runtime/bin/vmservice/cl...
File runtime/bin/vmservice/client/lib/src/elements/heap_profile.dart (right):

https://codereview.chromium.org/342513004/diff/40001/runtime/bin/vmservice/cl...
runtime/bin/vmservice/client/lib/src/elements/heap_profile.dart:81: new
SortedTableColumn(''),
Why this empty column?

https://codereview.chromium.org/342513004/diff/40001/runtime/bin/vmservice/cl...
runtime/bin/vmservice/client/lib/src/elements/heap_profile.dart:101:
classTable.sortColumnIndex = 2;
Remind the reader which column this is.

https://codereview.chromium.org/342513004/diff/40001/runtime/bin/vmservice/cl...
runtime/bin/vmservice/client/lib/src/elements/heap_profile.dart:129: void
_updateClassStats(Class cls, List newSpace, List oldSpace) {
Any way to share more code between new and old? It should be fairly uniform, and
maybe someday there will be a third one ;)

https://codereview.chromium.org/342513004/diff/40001/runtime/bin/vmservice/cl...
File runtime/bin/vmservice/client/lib/src/service/object.dart (right):

https://codereview.chromium.org/342513004/diff/40001/runtime/bin/vmservice/cl...
runtime/bin/vmservice/client/lib/src/service/object.dart:193: @observable
DateTime date;
Please choose a more descriptive name. lastUpdated?

https://codereview.chromium.org/342513004/diff/40001/runtime/bin/vmservice/cl...
runtime/bin/vmservice/client/lib/src/service/object.dart:665: @observable int
newHeapUsed = 0;
The same three fields are repeated for new and old. Consider creating an
abstraction.

https://codereview.chromium.org/342513004/diff/40001/runtime/bin/vmservice/cl...
runtime/bin/vmservice/client/lib/src/service/object.dart:986: class
AllocationStat extends Observable {
Consider adding pointer to the C++ counterpart in class_table.h.

https://codereview.chromium.org/342513004/diff/40001/runtime/vm/class_table.cc
File runtime/vm/class_table.cc (right):

https://codereview.chromium.org/342513004/diff/40001/runtime/vm/class_table.c...
runtime/vm/class_table.cc:341: "dateLastGC",
Consider renaming GC -> ServiceGC or something to indicate that you are only
considering GC explicitly requested from the service.

https://codereview.chromium.org/342513004/diff/40001/runtime/vm/isolate.h
File runtime/vm/isolate.h (right):

https://codereview.chromium.org/342513004/diff/40001/runtime/vm/isolate.h#new...
runtime/vm/isolate.h:464: void timestampLastAllocationProfileAccumulatorReset()
{
Naming convention: start with uppercase.

Also, please add a prefix like "Update" or "Set" to the mutators, to distinguish
them more from the accessors.

https://codereview.chromium.org/342513004/diff/40001/runtime/vm/isolate.h#new...
runtime/vm/isolate.h:473: void timestampLastAllocationProfileGC() {
Ditto.

https://codereview.chromium.org/342513004/diff/40001/runtime/vm/isolate.h#new...
runtime/vm/isolate.h:623: // Timestamps of last operation via service.
Longer-term, I wish we had a different way of associating auxiliary info with an
isolate, rather than adding more fields.

https://codereview.chromium.org/342513004/diff/40001/runtime/vm/service.cc
File runtime/vm/service.cc (right):

https://codereview.chromium.org/342513004/diff/40001/runtime/vm/service.cc#ne...
runtime/vm/service.cc:1943: jsobj.AddPropertyF("date", "%" Pd64 "",
OS::GetCurrentTimeMillis());
It's potentially confusing that 'date' is in milliseconds and 'uptime' is in
seconds. Consider adding suffixes to all durations/timestamps, or standardize on
one unit.

[Cutch, 2014-06-18 14:35:04]

https://codereview.chromium.org/342513004/diff/40001/runtime/bin/vmservice/cl...
File runtime/bin/vmservice/client/lib/src/app/view_model.dart (right):

https://codereview.chromium.org/342513004/diff/40001/runtime/bin/vmservice/cl...
runtime/bin/vmservice/client/lib/src/app/view_model.dart:137: //print('$label
took ${sw.elapsedMicroseconds} us.');
On 2014/06/17 21:04:06, koda wrote:
> Uncomment or remove.

Done.

https://codereview.chromium.org/342513004/diff/40001/runtime/bin/vmservice/cl...
File runtime/bin/vmservice/client/lib/src/elements/heap_profile.dart (right):

https://codereview.chromium.org/342513004/diff/40001/runtime/bin/vmservice/cl...
runtime/bin/vmservice/client/lib/src/elements/heap_profile.dart:81: new
SortedTableColumn(''),
On 2014/06/17 21:04:06, koda wrote:
> Why this empty column?

Spacer column, added comment.

https://codereview.chromium.org/342513004/diff/40001/runtime/bin/vmservice/cl...
runtime/bin/vmservice/client/lib/src/elements/heap_profile.dart:101:
classTable.sortColumnIndex = 2;
On 2014/06/17 21:04:06, koda wrote:
> Remind the reader which column this is.

Done.

https://codereview.chromium.org/342513004/diff/40001/runtime/bin/vmservice/cl...
runtime/bin/vmservice/client/lib/src/elements/heap_profile.dart:129: void
_updateClassStats(Class cls, List newSpace, List oldSpace) {
On 2014/06/17 21:04:06, koda wrote:
> Any way to share more code between new and old? It should be fairly uniform,
and
> maybe someday there will be a third one ;)

Done.

https://codereview.chromium.org/342513004/diff/40001/runtime/bin/vmservice/cl...
File runtime/bin/vmservice/client/lib/src/service/object.dart (right):

https://codereview.chromium.org/342513004/diff/40001/runtime/bin/vmservice/cl...
runtime/bin/vmservice/client/lib/src/service/object.dart:193: @observable
DateTime date;
On 2014/06/17 21:04:07, koda wrote:
> Please choose a more descriptive name. lastUpdated?

Done.

https://codereview.chromium.org/342513004/diff/40001/runtime/bin/vmservice/cl...
runtime/bin/vmservice/client/lib/src/service/object.dart:665: @observable int
newHeapUsed = 0;
On 2014/06/17 21:04:06, koda wrote:
> The same three fields are repeated for new and old. Consider creating an
> abstraction.

Done.

https://codereview.chromium.org/342513004/diff/40001/runtime/bin/vmservice/cl...
runtime/bin/vmservice/client/lib/src/service/object.dart:986: class
AllocationStat extends Observable {
On 2014/06/17 21:04:06, koda wrote:
> Consider adding pointer to the C++ counterpart in class_table.h.

Done.

https://codereview.chromium.org/342513004/diff/40001/runtime/vm/class_table.cc
File runtime/vm/class_table.cc (right):

https://codereview.chromium.org/342513004/diff/40001/runtime/vm/class_table.c...
runtime/vm/class_table.cc:341: "dateLastGC",
On 2014/06/17 21:04:07, koda wrote:
> Consider renaming GC -> ServiceGC or something to indicate that you are only
> considering GC explicitly requested from the service.

Done.

https://codereview.chromium.org/342513004/diff/40001/runtime/vm/isolate.h
File runtime/vm/isolate.h (right):

https://codereview.chromium.org/342513004/diff/40001/runtime/vm/isolate.h#new...
runtime/vm/isolate.h:464: void timestampLastAllocationProfileAccumulatorReset()
{
On 2014/06/17 21:04:07, koda wrote:
> Naming convention: start with uppercase.
> 
> Also, please add a prefix like "Update" or "Set" to the mutators, to
distinguish
> them more from the accessors.

Done.

https://codereview.chromium.org/342513004/diff/40001/runtime/vm/isolate.h#new...
runtime/vm/isolate.h:473: void timestampLastAllocationProfileGC() {
On 2014/06/17 21:04:07, koda wrote:
> Ditto.

Done.

https://codereview.chromium.org/342513004/diff/40001/runtime/vm/isolate.h#new...
runtime/vm/isolate.h:623: // Timestamps of last operation via service.
On 2014/06/17 21:04:07, koda wrote:
> Longer-term, I wish we had a different way of associating auxiliary info with
an
> isolate, rather than adding more fields.

I had the same thought. Agreed.

[koda, 2014-06-18 17:04:02]

Just two more suggested readability nits...

https://codereview.chromium.org/342513004/diff/80001/runtime/bin/vmservice/cl...
File runtime/bin/vmservice/client/lib/src/elements/heap_profile.dart (right):

https://codereview.chromium.org/342513004/diff/80001/runtime/bin/vmservice/cl...
runtime/bin/vmservice/client/lib/src/elements/heap_profile.dart:224: cell.text =
classTable.getFormattedValue(rowIndex, 10);
Consider having a constant SPACER_COLUMNS = [1, 6] and then implementing this
and the above method with a loop instead. That would make make changing the
spacing less error-prone.

https://codereview.chromium.org/342513004/diff/80001/runtime/bin/vmservice/cl...
File runtime/bin/vmservice/client/lib/src/service/object.dart (right):

https://codereview.chromium.org/342513004/diff/80001/runtime/bin/vmservice/cl...
runtime/bin/vmservice/client/lib/src/service/object.dart:492: class IsolateHeap
extends Observable {
To nit, I think either "space" or "generation" would be a less ambiguous term
than "heap" (and you already call the instances newSpace/oldSpace). I think of
the heap as the basic abstraction and the spaces and generations as
implementation details.

[Cutch, 2014-06-18 17:13:04]

https://codereview.chromium.org/342513004/diff/80001/runtime/bin/vmservice/cl...
File runtime/bin/vmservice/client/lib/src/elements/heap_profile.dart (right):

https://codereview.chromium.org/342513004/diff/80001/runtime/bin/vmservice/cl...
runtime/bin/vmservice/client/lib/src/elements/heap_profile.dart:224: cell.text =
classTable.getFormattedValue(rowIndex, 10);
On 2014/06/18 17:04:02, koda wrote:
> Consider having a constant SPACER_COLUMNS = [1, 6] and then implementing this
> and the above method with a loop instead. That would make make changing the
> spacing less error-prone.

Done.

https://codereview.chromium.org/342513004/diff/80001/runtime/bin/vmservice/cl...
File runtime/bin/vmservice/client/lib/src/service/object.dart (right):

https://codereview.chromium.org/342513004/diff/80001/runtime/bin/vmservice/cl...
runtime/bin/vmservice/client/lib/src/service/object.dart:492: class IsolateHeap
extends Observable {
On 2014/06/18 17:04:02, koda wrote:
> To nit, I think either "space" or "generation" would be a less ambiguous term
> than "heap" (and you already call the instances newSpace/oldSpace). I think of
> the heap as the basic abstraction and the spaces and generations as
> implementation details.

Done. HeapSpace.

[koda, 2014-06-18 17:15:45]

lgtm

[Cutch, 2014-06-18 17:40:47]

Committed patchset #7 manually as r37456 (presubmit successful).