Fix --track_gc_object_stats option.

Fix --track_gc_object_stats option.

 - Update ObjectStatsVisitTracker::Visit function to check if CodeCache is of
   CodeCache type, and extract the FixedArray from the struct if so
 - Fix typo in v8-counters.h where count_of_FIXED_ARRAY_XXX fields weren't being
   initialized.

BUG=v8:2780
R=danno@chromium.org, hpayer@chromium.org

Committed: http://code.google.com/p/v8/source/detail?r=15748

[danno, 2013-07-16 08:02:30]

DBCs...

https://codereview.chromium.org/19257002/diff/3001/src/mark-compact.cc
File src/mark-compact.cc (right):

https://codereview.chromium.org/19257002/diff/3001/src/mark-compact.cc#newcod...
src/mark-compact.cc:1637: }
The previous for lines are unneeded (and wrong).

https://codereview.chromium.org/19257002/diff/3001/src/mark-compact.cc#newcod...
src/mark-compact.cc:1638: if (code_cache != heap->empty_descriptor_array()) {
Just preface the code above with "code_cache->IsFixedArray() &&"

[danno, 2013-07-16 08:21:50]

https://codereview.chromium.org/19257002/diff/3001/src/mark-compact.cc
File src/mark-compact.cc (right):

https://codereview.chromium.org/19257002/diff/3001/src/mark-compact.cc#newcod...
src/mark-compact.cc:1637: }
On 2013/07/16 08:02:30, danno wrote:
> The previous for lines are unneeded (and wrong).

Sorry, ignore my previous comment. I mis-read the code. Your solution is
actually more or less OK, but it's confusing because it manipulates FixedArrays
masquerading as a code cache, and it erroneously uses empty_descriptor_array in
place of empty_fixed_array.

I would prefer:

if (map_obj->has_code_cache()) {
  CodeCache* cache = CodeCache::cast(map_obj->code_cache());
  heap->RecordObjectStats(
     FIXED_ARRAY_TYPE,
     MAP_CODE_CACHE_SUB_TYPE,
     cache->default_cache()->Size());

  // ... for extra points, account for allocations attributable to
  //  cache->normal_cache(), probably something like:
  if (!cache->normal_type_cache()->IsUndefined()) {
    heap->RecordObjectStats(
       FIXED_ARRAY_TYPE,
       MAP_CODE_CACHE_SUB_TYPE,
       FixedArray::cast(cache->normal_type_cache())->Size());
  }
}

You will have to implement has_code_cache in object-inl.h:

bool has_code_cache() {
  return code_cache() != GetIsolate()->heap()->empty_fixed_array());
}

https://codereview.chromium.org/19257002/diff/3001/src/mark-compact.cc#newcod...
src/mark-compact.cc:1638: if (code_cache != heap->empty_descriptor_array()) {
On 2013/07/16 08:02:30, danno wrote:
> Just preface the code above with "code_cache->IsFixedArray() &&"

Ignore this comment.

[rmcilroy, 2013-07-16 12:05:58]

Thanks for the comments!  PTAL

https://codereview.chromium.org/19257002/diff/3001/src/mark-compact.cc
File src/mark-compact.cc (right):

https://codereview.chromium.org/19257002/diff/3001/src/mark-compact.cc#newcod...
src/mark-compact.cc:1637: }
On 2013/07/16 08:21:50, danno wrote:
> On 2013/07/16 08:02:30, danno wrote:
> > The previous for lines are unneeded (and wrong).
> 
> Sorry, ignore my previous comment. I mis-read the code. Your solution is
> actually more or less OK, but it's confusing because it manipulates
FixedArrays
> masquerading as a code cache, and it erroneously uses empty_descriptor_array
in
> place of empty_fixed_array.
> 
> I would prefer:
> 
> if (map_obj->has_code_cache()) {
>   CodeCache* cache = CodeCache::cast(map_obj->code_cache());
>   heap->RecordObjectStats(
>      FIXED_ARRAY_TYPE,
>      MAP_CODE_CACHE_SUB_TYPE,
>      cache->default_cache()->Size());
> 
>   // ... for extra points, account for allocations attributable to
>   //  cache->normal_cache(), probably something like:
>   if (!cache->normal_type_cache()->IsUndefined()) {
>     heap->RecordObjectStats(
>        FIXED_ARRAY_TYPE,
>        MAP_CODE_CACHE_SUB_TYPE,
>        FixedArray::cast(cache->normal_type_cache())->Size());
>   }
> }
> 
> You will have to implement has_code_cache in object-inl.h:
> 
> bool has_code_cache() {
>   return code_cache() != GetIsolate()->heap()->empty_fixed_array());
> }


Ahh I see. I was confused because I couldn't always cast the code_cache object
to a CodeCache and assumed it was sometimes still a FixedArray, but I see from
this that it is only a FixedArray object when it is empty (i.e., when it is
empty_fixed_array()).

Done as suggested.

https://codereview.chromium.org/19257002/diff/3001/src/mark-compact.cc#newcod...
src/mark-compact.cc:1638: if (code_cache != heap->empty_descriptor_array()) {
On 2013/07/16 08:21:50, danno wrote:
> On 2013/07/16 08:02:30, danno wrote:
> > Just preface the code above with "code_cache->IsFixedArray() &&"
> 
> Ignore this comment.

Ack

[danno, 2013-07-16 14:20:34]

almost there, please remove the stray whitespace change,

https://codereview.chromium.org/19257002/diff/8001/src/objects-inl.h
File src/objects-inl.h (left):

https://codereview.chromium.org/19257002/diff/8001/src/objects-inl.h#oldcode4347
src/objects-inl.h:4347: 
Can you please remove this whitespace change?

[Hannes Payer (out of office), 2013-07-16 14:39:28]

https://chromiumcodereview.appspot.com/19257002/diff/8001/src/objects-inl.h
File src/objects-inl.h (right):

https://chromiumcodereview.appspot.com/19257002/diff/8001/src/objects-inl.h#n...
src/objects-inl.h:3608: 
add newline

https://chromiumcodereview.appspot.com/19257002/diff/8001/src/objects-inl.h#n...
src/objects-inl.h:3612: 
add newline

https://chromiumcodereview.appspot.com/19257002/diff/8001/src/objects.h
File src/objects.h (right):

https://chromiumcodereview.appspot.com/19257002/diff/8001/src/objects.h#newco...
src/objects.h:5310: // Returns true if map has an non-empty stub code cache.
"a non-empty stub..."

[rmcilroy_google, 2013-07-16 15:03:01]

https://codereview.chromium.org/19257002/diff/8001/src/objects-inl.h
File src/objects-inl.h (left):

https://codereview.chromium.org/19257002/diff/8001/src/objects-inl.h#oldcode4347
src/objects-inl.h:4347: 
On 2013/07/16 14:20:34, danno wrote:
> Can you please remove this whitespace change?

Opps, Done.

https://codereview.chromium.org/19257002/diff/8001/src/objects-inl.h
File src/objects-inl.h (right):

https://codereview.chromium.org/19257002/diff/8001/src/objects-inl.h#newcode3608
src/objects-inl.h:3608: 
On 2013/07/16 14:39:28, Hannes Payer wrote:
> add newline

Done.

https://codereview.chromium.org/19257002/diff/8001/src/objects-inl.h#newcode3612
src/objects-inl.h:3612: 
On 2013/07/16 14:39:28, Hannes Payer wrote:
> add newline

Done.

https://codereview.chromium.org/19257002/diff/8001/src/objects.h
File src/objects.h (right):

https://codereview.chromium.org/19257002/diff/8001/src/objects.h#newcode5310
src/objects.h:5310: // Returns true if map has an non-empty stub code cache.
On 2013/07/16 14:39:28, Hannes Payer wrote:
> "a non-empty stub..."

Done.

[danno, 2013-07-17 09:15:14]

lgtm

[Hannes Payer (out of office), 2013-07-18 08:55:23]

lgtm

[rmcilroy, 2013-07-18 09:22:37]

On 2013/07/18 08:55:23, Hannes Payer wrote:
> lgtm

Could one of you land this for me please (I guess I can't do it without
committer access?)

[danno, 2013-07-18 13:00:43]

Done.

[danno, 2013-07-18 13:00:47]

Committed patchset #4 manually as r15748 (presubmit successful).