Report "hidden properties" in heap profiler for fast properties case

Report "hidden properties" in heap profiler for fast properties case

BUG=v8:2212
TEST=cctest/test-heap-profiler/HiddenPropertiesFastCase
Committed: https://code.google.com/p/v8/source/detail?r=11966

[Vyacheslav Egorov (Google), 2012-07-02 12:12:45]

lgtm

http://codereview.chromium.org/10692058/diff/1/src/profile-generator.cc
File src/profile-generator.cc (right):

http://codereview.chromium.org/10692058/diff/1/src/profile-generator.cc#newco...
src/profile-generator.cc:2189: if (k->length() > 0) {
Consider comparing against Heap::hidden_symbol()

http://codereview.chromium.org/10692058/diff/1/src/profile-generator.cc#newco...
src/profile-generator.cc:2196: OS::Print("Empty name for fast property 1\n");
remove OS::Print

http://codereview.chromium.org/10692058/diff/1/src/profile-generator.cc#newco...
src/profile-generator.cc:2205: if (k->length() > 0) {
Consider comparing against Heap::hidden_symbol

http://codereview.chromium.org/10692058/diff/1/src/profile-generator.cc#newco...
src/profile-generator.cc:2208: OS::Print("Empty name for fast property 2\n");
remove OS::Print

[mnaganov (inactive), 2012-07-02 12:16:19]

LGTM

In the CL description, change "BUG=..." to "BUG=v8:..." otherwise it references
a Chromium bug.

http://codereview.chromium.org/10692058/diff/1/test/cctest/test-heap-profiler.cc
File test/cctest/test-heap-profiler.cc (right):

http://codereview.chromium.org/10692058/diff/1/test/cctest/test-heap-profiler...
test/cctest/test-heap-profiler.cc:1473: snapshot =
v8::HeapProfiler::TakeSnapshot(v8_str("HiddenPropertiesFastCase2"));
nit: 80 chars

http://codereview.chromium.org/10692058/diff/1/test/cctest/test-heap-profiler...
test/cctest/test-heap-profiler.cc:1477: hidden_props = GetProperty(c,
v8::HeapGraphEdge::kInternal, "hidden_properties");
nit: 80 chars

[alexeif, 2012-07-02 12:38:03]

http://codereview.chromium.org/10692058/diff/1/src/profile-generator.cc
File src/profile-generator.cc (right):

http://codereview.chromium.org/10692058/diff/1/src/profile-generator.cc#newco...
src/profile-generator.cc:2257: if (String::cast(k)->length() > 0) {
Please consider comparing against Heap::hidden_symbol here as well.

[yurys, 2012-07-02 12:43:26]

http://codereview.chromium.org/10692058/diff/1/src/profile-generator.cc
File src/profile-generator.cc (right):

http://codereview.chromium.org/10692058/diff/1/src/profile-generator.cc#newco...
src/profile-generator.cc:2189: if (k->length() > 0) {
On 2012/07/02 12:12:45, Vyacheslav Egorov (Google) wrote:
> Consider comparing against Heap::hidden_symbol()

Done.

http://codereview.chromium.org/10692058/diff/1/src/profile-generator.cc#newco...
src/profile-generator.cc:2196: OS::Print("Empty name for fast property 1\n");
On 2012/07/02 12:12:45, Vyacheslav Egorov (Google) wrote:
> remove OS::Print

Done.

http://codereview.chromium.org/10692058/diff/1/src/profile-generator.cc#newco...
src/profile-generator.cc:2205: if (k->length() > 0) {
On 2012/07/02 12:12:45, Vyacheslav Egorov (Google) wrote:
> Consider comparing against Heap::hidden_symbol

Done.

http://codereview.chromium.org/10692058/diff/1/src/profile-generator.cc#newco...
src/profile-generator.cc:2208: OS::Print("Empty name for fast property 2\n");
On 2012/07/02 12:12:45, Vyacheslav Egorov (Google) wrote:
> remove OS::Print

Done.

http://codereview.chromium.org/10692058/diff/1/test/cctest/test-heap-profiler.cc
File test/cctest/test-heap-profiler.cc (right):

http://codereview.chromium.org/10692058/diff/1/test/cctest/test-heap-profiler...
test/cctest/test-heap-profiler.cc:1473: snapshot =
v8::HeapProfiler::TakeSnapshot(v8_str("HiddenPropertiesFastCase2"));
On 2012/07/02 12:16:19, Mikhail Naganov (Chromium) wrote:
> nit: 80 chars

Done.

http://codereview.chromium.org/10692058/diff/1/test/cctest/test-heap-profiler...
test/cctest/test-heap-profiler.cc:1477: hidden_props = GetProperty(c,
v8::HeapGraphEdge::kInternal, "hidden_properties");
On 2012/07/02 12:16:19, Mikhail Naganov (Chromium) wrote:
> nit: 80 chars

Done.

[yurys, 2012-07-02 12:45:51]

http://codereview.chromium.org/10692058/diff/1/src/profile-generator.cc
File src/profile-generator.cc (right):

http://codereview.chromium.org/10692058/diff/1/src/profile-generator.cc#newco...
src/profile-generator.cc:2257: if (String::cast(k)->length() > 0) {
On 2012/07/02 12:38:03, alexeif wrote:
> Please consider comparing against Heap::hidden_symbol here as well.

Done.