- Base JSON stream printing on stack objects.

- Base JSON stream printing on stack objects.
- Simplify creation of a JSONStream.

- Add experimental coverage output to the VM.

R=asiva@google.com, johnmccutchan@google.com

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

[Ivan Posva, 2013-09-09 18:01:45]

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

https://codereview.chromium.org/23875015/diff/1/runtime/vm/coverage.cc#newcode55
runtime/vm/coverage.cc:55: // OS::Print("  %s -> %"Pd" @ %"Pd" %"Pd"\n",
Debugging code to be removed.

[Cutch, 2013-09-09 18:35:39]

lgtm

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

https://codereview.chromium.org/23875015/diff/1/runtime/vm/heap_histogram.cc#...
runtime/vm/heap_histogram.cc:166: jsarr.AddValue("count");
Because of how web_ui <template> tags are executed I needed to have the list of
property keys (aside from name and category) listed. If a new key is added to
"properties" the UI would display a table for that property.

https://codereview.chromium.org/23875015/diff/1/runtime/vm/heap_histogram.cc#...
runtime/vm/heap_histogram.cc:190: // jsobj.AddProperty("category", lib);
Yes, and the class above too (instead of printing it's name, print a reference.)

[siva, 2013-09-09 22:45:25]

lgtm

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

https://codereview.chromium.org/23875015/diff/1/runtime/vm/heap_histogram.cc#...
runtime/vm/heap_histogram.cc:199: JSONObject jsobj1(jsobj, "sums");
I would name this as aggregate_jsobj and some other more descriptive name than
jsobj1.

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

https://codereview.chromium.org/23875015/diff/1/runtime/vm/json_stream.cc#new...
runtime/vm/json_stream.cc:246: free(p);
Might make sense to abstract the common code in
AddFormattedValue and AddFormattedProperty into
a private method
JSONArray::AddValue(const char* format, ...)

https://codereview.chromium.org/23875015/diff/1/runtime/vm/object.h
File runtime/vm/object.h (right):

https://codereview.chromium.org/23875015/diff/1/runtime/vm/object.h#newcode130
runtime/vm/object.h:130: return ref ? "@"#object : ""#object;                   
                  \
format error in '\'

[Ivan Posva, 2013-09-10 17:21:31]

Thanks!

-Ivan

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

https://codereview.chromium.org/23875015/diff/1/runtime/vm/heap_histogram.cc#...
runtime/vm/heap_histogram.cc:166: jsarr.AddValue("count");
On 2013/09/09 18:35:39, Cutch wrote:
> Because of how web_ui <template> tags are executed I needed to have the list
of
> property keys (aside from name and category) listed. If a new key is added to
> "properties" the UI would display a table for that property.

Thanks for the explanation. Will this change with the switch to polymer?

https://codereview.chromium.org/23875015/diff/1/runtime/vm/heap_histogram.cc#...
runtime/vm/heap_histogram.cc:190: // jsobj.AddProperty("category", lib);
On 2013/09/09 18:35:39, Cutch wrote:
> Yes, and the class above too (instead of printing it's name, print a
reference.)

Done. But left the old code in so that the UI does not break.

https://codereview.chromium.org/23875015/diff/1/runtime/vm/heap_histogram.cc#...
runtime/vm/heap_histogram.cc:199: JSONObject jsobj1(jsobj, "sums");
On 2013/09/09 22:45:26, siva wrote:
> I would name this as aggregate_jsobj and some other more descriptive name than
> jsobj1.

Done.

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

https://codereview.chromium.org/23875015/diff/1/runtime/vm/json_stream.cc#new...
runtime/vm/json_stream.cc:246: free(p);
On 2013/09/09 22:45:26, siva wrote:
> Might make sense to abstract the common code in
> AddFormattedValue and AddFormattedProperty into
> a private method
> JSONArray::AddValue(const char* format, ...) 

I would like to add support for an AddValueV in addition to an AddValueF
function (similar in JSONObject). At that point we should also consolidate these
two functions.

https://codereview.chromium.org/23875015/diff/1/runtime/vm/object.h
File runtime/vm/object.h (right):

https://codereview.chromium.org/23875015/diff/1/runtime/vm/object.h#newcode130
runtime/vm/object.h:130: return ref ? "@"#object : ""#object;                   
                  \
On 2013/09/09 22:45:26, siva wrote:
> format error in '\'

Done.

[Ivan Posva, 2013-09-10 17:22:11]

Committed patchset #2 manually as r27344 (presubmit successful).

[Cutch, 2013-09-10 17:49:03]

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

https://codereview.chromium.org/23875015/diff/1/runtime/vm/heap_histogram.cc#...
runtime/vm/heap_histogram.cc:166: jsarr.AddValue("count");
On 2013/09/10 17:21:31, Ivan Posva wrote:
> On 2013/09/09 18:35:39, Cutch wrote:
> > Because of how web_ui <template> tags are executed I needed to have the list
> of
> > property keys (aside from name and category) listed. If a new key is added
to
> > "properties" the UI would display a table for that property.
> 
> Thanks for the explanation. Will this change with the switch to polymer?

Not sure yet. Polymer looks like it's going to require more Dart code so I may
end up being able to extract the list of properties from the individual
elements, in that case this will go away.