Fix --track_gc_object_stats option.

src/mark-compact.cc

 // Copyright 2012 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 1613 matching lines @@
       heap->RecordObjectStats(FIXED_ARRAY_TYPE,
                               DESCRIPTOR_ARRAY_SUB_TYPE,
                               fixed_array_size);
     }
     if (map_obj->HasTransitionArray()) {
       int fixed_array_size = map_obj->transitions()->Size();
       heap->RecordObjectStats(FIXED_ARRAY_TYPE,
                               TRANSITION_ARRAY_SUB_TYPE,
                               fixed_array_size);
     }
-    if (map_obj->code_cache() != heap->empty_fixed_array()) {
+    Object* code_cache = map_obj->code_cache();
+    if (!code_cache->IsFixedArray()) {
+      code_cache = CodeCache::cast(code_cache)->default_cache();
+    }

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

The previous for lines are unneeded (and wrong).

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

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());
}

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

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.

+    if (code_cache != heap->empty_descriptor_array()) {

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

Just preface the code above with "code_cache->IsFixedArray() &&"

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

Ignore this comment.

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

Ack

       heap->RecordObjectStats(
           FIXED_ARRAY_TYPE,
           MAP_CODE_CACHE_SUB_TYPE,
-          FixedArray::cast(map_obj->code_cache())->Size());
+          FixedArray::cast(code_cache)->Size());
     }
     ObjectStatsVisitBase(kVisitMap, map, obj);
   }
 };
 
 
 template<>
 class MarkCompactMarkingVisitor::ObjectStatsTracker<
     MarkCompactMarkingVisitor::kVisitCode> {
  public:
@@ skipping 2666 matching lines @@
   while (buffer != NULL) {
     SlotsBuffer* next_buffer = buffer->next();
     DeallocateBuffer(buffer);
     buffer = next_buffer;
   }
   *buffer_address = NULL;
 }
 
 
 } }  // namespace v8::internal