Improved, safer handling of the symbol table. The symbols themselves...

src/mark-compact.cc

 // Copyright 2006-2008 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 544 matching lines @@
 }
 
 
 bool MarkCompactCollector::MustBeMarked(Object** p) {
   // Check whether *p is a HeapObject pointer.
   if (!(*p)->IsHeapObject()) return false;
   return !HeapObject::cast(*p)->IsMarked();
 }
 
 
-// Helper class to unmark marked objects in a range of pointers but
-// not recursively.
-class UnmarkingVisitor : public ObjectVisitor {
+class SymbolMarkingVisitor : public ObjectVisitor {
  public:
   void VisitPointers(Object** start, Object** end) {
+    MarkingVisitor marker;
     for (Object** p = start; p < end; p++) {
-      if ((*p)->IsHeapObject() && HeapObject::cast(*p)->IsMarked()) {
-        MarkCompactCollector::ClearMark(HeapObject::cast(*p));
-      }
+      if (!(*p)->IsHeapObject()) continue;

[iposva, 2009/05/05 05:05:19]

Can there be non-heap entries in the symbol table? I guess the table is
initialized to zeroes, but it would be nice to comment it here, so that I and
others don't have to figure that out everytime we read this code.

+
+      HeapObject* object = HeapObject::cast(*p);

[Kasper Lund, 2009/05/05 05:22:34]

The newlines here look weird. How about moving the HeapObject* object definition
below the comment?

[Kevin Millikin (Chromium), 2009/05/05 05:34:07]

I could do that, but the comment goes with the continue.

+      // If the object is marked, we have marked or are in the process
+      // of marking subparts.
+      if (object->IsMarked()) continue;
+
+      // The object is unmarked, we do not need to unmark to use its
+      // map.
+      Map* map = object->map();
+      object->IterateBody(map->instance_type(),

[Kasper Lund, 2009/05/05 05:22:34]

I'd compute the size before the IterateBody call, stuff it in a local variable,
and use it to get nicely formatted code where the IterateBody call fits on one
line. But that's just me.

[Kevin Millikin (Chromium), 2009/05/05 05:34:07]

That's not the way I roll.

+                          object->SizeFromMap(map),
+                          &marker);
     }
   }
 };
 
 
-void MarkCompactCollector::ProcessRoots(RootMarkingVisitor* visitor) {
-  // Handle the symbol table specially.  Mark the prefix and the
-  // symbol table itself.  Do not mark the symbol table entries, but
-  // do explicitly mark all other objects reachable from them.
-  //
+void MarkCompactCollector::MarkSymbolTable() {
   // Objects reachable from symbols are marked as live so as to ensure
   // that if the symbol itself remains alive after GC for any reason,
   // and if it is a sliced string or a cons string backed by an
   // external string (even indirectly), then the external string does
   // not receive a weak reference callback.
   SymbolTable* symbol_table = SymbolTable::cast(Heap::symbol_table());
-  // First mark everything reachable from the symbol table, then
-  // unmark just the elements themselves.
-  symbol_table->Iterate(visitor);
-  // There may be overflowed objects in the heap.  Visit them now.
-  while (marking_stack.overflowed()) {
-    RefillMarkingStack();
-    EmptyMarkingStack(visitor->stack_visitor());
-  }
-  UnmarkingVisitor unmarking_visitor;
-  symbol_table->IterateElements(&unmarking_visitor);
   // Mark the symbol table itself.
   SetMark(symbol_table);
+  // Explicitly mark the prefix.
+  MarkingVisitor marker;
+  symbol_table->IteratePrefix(&marker);
+  ProcessMarkingStack(&marker);
+  // Mark subparts of the symbols but not the symbols themselves
+  // (unless reachable from another symbol).
+  SymbolMarkingVisitor symbol_marker;
+  symbol_table->IterateElements(&symbol_marker);
+  ProcessMarkingStack(&marker);
+}
 
+
+void MarkCompactCollector::MarkRoots(RootMarkingVisitor* visitor) {
   // Mark the heap roots including global variables, stack variables,
   // etc., and all objects reachable from them.
   Heap::IterateStrongRoots(visitor);
 
+  // Handle the symbol table specially.
+  MarkSymbolTable();
+
   // There may be overflowed objects in the heap.  Visit them now.
   while (marking_stack.overflowed()) {
     RefillMarkingStack();
     EmptyMarkingStack(visitor->stack_visitor());
   }
 }
 
 
 void MarkCompactCollector::MarkObjectGroups() {
   List<ObjectGroup*>* object_groups = GlobalHandles::ObjectGroups();
@@ skipping 120 matching lines @@
   state_ = MARK_LIVE_OBJECTS;
 #endif
   // The to space contains live objects, the from space is used as a marking
   // stack.
   marking_stack.Initialize(Heap::new_space()->FromSpaceLow(),
                            Heap::new_space()->FromSpaceHigh());
 
   ASSERT(!marking_stack.overflowed());
 
   RootMarkingVisitor root_visitor;
-  ProcessRoots(&root_visitor);
+  MarkRoots(&root_visitor);
 
   // The objects reachable from the roots are marked black, unreachable
   // objects are white.  Mark objects reachable from object groups with at
   // least one marked object, and continue until no new objects are
   // reachable from the object groups.
   ProcessObjectGroups(root_visitor.stack_visitor());
 
   // The objects reachable from the roots or object groups are marked black,
   // unreachable objects are white.  Process objects reachable only from
   // weak global handles.
@@ skipping 1028 matching lines @@
 
 void MarkCompactCollector::RebuildRSets() {
 #ifdef DEBUG
   ASSERT(state_ == RELOCATE_OBJECTS);
   state_ = REBUILD_RSETS;
 #endif
   Heap::RebuildRSets();
 }
 
 } }  // namespace v8::internal