Provide more accurate results about used heap size via GetHeapStatistics.

src/heap.cc

Index: src/heap.cc
diff --git a/src/heap.cc b/src/heap.cc
index 134f40e5070cf80d41978653d7406120d28a58fa..5989168a77e8d076da2f8545b7658e214ccc408a 100644
--- a/src/heap.cc
+++ b/src/heap.cc
@@ -395,7 +395,7 @@ intptr_t Heap::SizeOfObjects() {
   intptr_t total = 0;
   AllSpaces spaces;
   for (Space* space = spaces.next(); space != NULL; space = spaces.next()) {
-    total += space->Size();
+    total += space->SizeOfObjects();
   }
   return total;
 }
@@ -4399,13 +4399,10 @@ void Heap::RecordStats(HeapStats* stats, bool take_snapshot) {
       MemoryAllocator::Size() + MemoryAllocator::Available();
   *stats->os_error = OS::GetLastError();
   if (take_snapshot) {
-    HeapIterator iterator;
+    HeapIterator iterator(HeapIterator::kPreciseFiltering);
     for (HeapObject* obj = iterator.next();
          obj != NULL;
          obj = iterator.next()) {
-      // Note: snapshot won't be precise because IsFreeListNode returns true
-      // for any bytearray.
-      if (FreeListNode::IsFreeListNode(obj)) continue;

[antonm, 2010/11/13 17:37:51]

thank you very much!

       InstanceType type = obj->map()->instance_type();
       ASSERT(0 <= type && type <= LAST_TYPE);
       stats->objects_per_type[type]++;
@@ -4760,7 +4757,17 @@ OldSpace* OldSpaces::next() {
 }
 
 
-SpaceIterator::SpaceIterator() : current_space_(FIRST_SPACE), iterator_(NULL) {
+SpaceIterator::SpaceIterator()
+    : current_space_(FIRST_SPACE),
+      iterator_(NULL),
+      size_func_(NULL) {
+}
+
+
+SpaceIterator::SpaceIterator(HeapObjectCallback size_func)
+    : current_space_(FIRST_SPACE),
+      iterator_(NULL),
+      size_func_(size_func) {
 }
 
 
@@ -4798,25 +4805,25 @@ ObjectIterator* SpaceIterator::CreateIterator() {
 
   switch (current_space_) {
     case NEW_SPACE:
-      iterator_ = new SemiSpaceIterator(Heap::new_space());
+      iterator_ = new SemiSpaceIterator(Heap::new_space(), size_func_);
       break;
     case OLD_POINTER_SPACE:
-      iterator_ = new HeapObjectIterator(Heap::old_pointer_space());
+      iterator_ = new HeapObjectIterator(Heap::old_pointer_space(), size_func_);
       break;
     case OLD_DATA_SPACE:
-      iterator_ = new HeapObjectIterator(Heap::old_data_space());
+      iterator_ = new HeapObjectIterator(Heap::old_data_space(), size_func_);
       break;
     case CODE_SPACE:
-      iterator_ = new HeapObjectIterator(Heap::code_space());
+      iterator_ = new HeapObjectIterator(Heap::code_space(), size_func_);
       break;
     case MAP_SPACE:
-      iterator_ = new HeapObjectIterator(Heap::map_space());
+      iterator_ = new HeapObjectIterator(Heap::map_space(), size_func_);
       break;
     case CELL_SPACE:
-      iterator_ = new HeapObjectIterator(Heap::cell_space());
+      iterator_ = new HeapObjectIterator(Heap::cell_space(), size_func_);
       break;
     case LO_SPACE:
-      iterator_ = new LargeObjectIterator(Heap::lo_space());
+      iterator_ = new LargeObjectIterator(Heap::lo_space(), size_func_);
       break;
   }
 
@@ -4826,7 +4833,64 @@ ObjectIterator* SpaceIterator::CreateIterator() {
 }
 
 
-HeapIterator::HeapIterator() {
+class FreeListNodesFilter : public Malloced {
+ public:
+  FreeListNodesFilter() {
+    MarkFreeListNodes();
+  }
+
+  inline bool IsFreeListNode(HeapObject* object) {
+    if (object->IsMarked()) {
+      object->ClearMark();
+      return true;
+    } else {
+      return false;
+    }
+  }
+
+  static int ObjectSize(HeapObject* object) {
+    bool marked = object->IsMarked();

[antonm, 2010/11/13 17:37:51]

MapWord might be the right abstraction for this code, see for example
CountMarkedCallback in mark-compact.cc.  BTW, you could probably just reuse it
here.

[mnaganov (inactive), 2010/11/13 19:05:30]

Hm. Not sure how using MapWord will help. HeapObject's 'IsMarked', 'SetMark',
and 'ClearMark' delegate to MapWord.

Why I need to have this function is because ObjectIterators need to get the
object size before they return pointer to it. And they will crash if map pointer
will be marked. And I need to restore marking back because I will need to check
it later via IsFreeListNode.

+    if (marked)

[antonm, 2010/11/13 17:37:51]

nit: either on a single line or in braces, please

[mnaganov (inactive), 2010/11/13 19:05:30]

Done.

+      object->ClearMark();
+    int size = object->Size();
+    if (marked)

[antonm, 2010/11/13 17:37:51]

ditto

[mnaganov (inactive), 2010/11/13 19:05:30]

Done.

+      object->SetMark();
+    return size;
+  }
+
+ private:
+  void MarkFreeListNodes() {
+    Heap::old_pointer_space()->MarkFreeListNodes();
+    Heap::old_data_space()->MarkFreeListNodes();
+    MarkCodeSpaceFreeListNodes();
+    Heap::map_space()->MarkFreeListNodes();
+    Heap::cell_space()->MarkFreeListNodes();
+  }
+
+  void MarkCodeSpaceFreeListNodes() {
+    // For code space, using FreeListNode::IsFreeListNode is OK.
+    ObjectIterator* iter = new HeapObjectIterator(Heap::code_space());

[antonm, 2010/11/13 17:37:51]

why you need to malloc iterator?  can you allocate it on the stack?

[mnaganov (inactive), 2010/11/13 19:05:30]

Not sure. It is a descendant of the class Malloced.

+    for (HeapObject* obj = iter->next_object();
+         obj != NULL;
+         obj = iter->next_object()) {
+      if (FreeListNode::IsFreeListNode(obj))

[antonm, 2010/11/13 17:37:51]

ditto for braces

[mnaganov (inactive), 2010/11/13 19:05:30]

Done.

+        obj->SetMark();
+    }
+    delete iter;
+  }
+};
+
+
+HeapIterator::HeapIterator()
+    : filtering_(HeapIterator::kNoFiltering),
+      filter_(NULL) {
+  Init();
+}
+
+
+HeapIterator::HeapIterator(HeapIterator::FreeListNodesFiltering filtering)
+    : filtering_(filtering),
+      filter_(NULL) {
   Init();
 }
 
@@ -4838,7 +4902,12 @@ HeapIterator::~HeapIterator() {
 
 void HeapIterator::Init() {
   // Start the iteration.
-  space_iterator_ = new SpaceIterator();
+  if (filtering_ == kPreciseFiltering) {
+    filter_ = new FreeListNodesFilter();

[antonm, 2010/11/13 17:37:51]

looks like new trend is to omit () for ctors with no args

[mnaganov (inactive), 2010/11/13 19:05:30]

Done.

+    space_iterator_ = new SpaceIterator(FreeListNodesFilter::ObjectSize);
+  } else {
+    space_iterator_ = new SpaceIterator();

[antonm, 2010/11/13 17:37:51]

ditto

[mnaganov (inactive), 2010/11/13 19:05:30]

Done.

+  }
   object_iterator_ = space_iterator_->next();
 }
 
@@ -4848,10 +4917,23 @@ void HeapIterator::Shutdown() {
   delete space_iterator_;
   space_iterator_ = NULL;
   object_iterator_ = NULL;
+  delete filter_;
+  filter_ = NULL;
 }
 
 
 HeapObject* HeapIterator::next() {
+  if (filter_ == NULL)

[antonm, 2010/11/13 17:37:51]

ditto for braces (here and below)

[mnaganov (inactive), 2010/11/13 19:05:30]

Done.

+    return NextObject();
+
+  HeapObject* obj = NextObject();
+  while (obj != NULL && filter_->IsFreeListNode(obj))
+    obj = NextObject();
+  return obj;
+}
+
+
+HeapObject* HeapIterator::NextObject() {
   // No iterator means we are done.
   if (object_iterator_ == NULL) return NULL;