More special handling of the symbol table during garbage collection....

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 {
+ public:
+  void VisitPointers(Object** start, Object** end) {
+    for (Object** p = start; p < end; p++) {
+      if ((*p)->IsHeapObject() && HeapObject::cast(*p)->IsMarked()) {
+        MarkCompactCollector::ClearMark(HeapObject::cast(*p));
+      }
+    }
+  }
+};
+
+
 void MarkCompactCollector::ProcessRoots(RootMarkingVisitor* visitor) {
-  // Mark the heap roots gray, including global variables, stack variables,
-  // etc.
+  // 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.
+  //
+  // 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.

[iposva, 2009/05/04 17:54:47]

In retrospect was that not always a problem even with regular non-external
strings only being referred to by a sliced string in the symbol table?

[Kevin Millikin (Chromium), 2009/05/04 18:34:52]

I don't think so.  The problem was that the weak reference callback was getting
called for the external string.  With no callback, there should not be any (err,
this) problem.

+  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);

[iposva, 2009/05/04 17:54:47]

The flipping of mark bits to marked and back to unmarked seems fragile to me.
Currently this only works because of the order (symbol table walking being
first.)

Since we need to handle the symbol table in a special way. The direct references
are not marked but any depending ones. Can we add a special marker instead? That
way we would not be depending on resetting mark bits during a GC, which is an
unneeded break of the assertions of the GC algorithm and adds unknown ordering
dependencies in the marking steps making the code more complex.

[Kevin Millikin (Chromium), 2009/05/04 18:34:52]

This was just the simplest implementation to get a fix out asap.  It leverages
all the existing infrastructure for recursive marking and handling (the
admittedly as close to impossible as possible) marking stack overflow. I agree
that it's dodgy.

+
+  // Mark the heap roots including global variables, stack variables,
+  // etc., and all objects reachable from them.
   Heap::IterateStrongRoots(visitor);
 
-  // Take care of the symbol table specially.
-  SymbolTable* symbol_table = SymbolTable::cast(Heap::symbol_table());
-  // 1. Mark the prefix of the symbol table gray.
-  symbol_table->IteratePrefix(visitor);
-  // 2. Mark the symbol table black (ie, do not push it on the marking stack
-  // or mark it overflowed).
-  SetMark(symbol_table);
-
   // 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 167 matching lines @@
 
 
 static int CountMarkedCallback(HeapObject* obj) {
   MapWord map_word = obj->map_word();
   map_word.ClearMark();
   return obj->SizeFromMap(map_word.ToMap());
 }
 
 
 #ifdef DEBUG
-void MarkCompactCollector::UpdateLiveObjectCount(HeapObject* obj) {
-  live_bytes_ += obj->Size();
+void MarkCompactCollector::UpdateLiveObjectCount(HeapObject* obj, int scale) {
+  ASSERT(scale == -1 || scale == 1);
+  live_bytes_ += obj->Size() * scale;
   if (Heap::new_space()->Contains(obj)) {
-    live_young_objects_++;
+    live_young_objects_ += scale;
   } else if (Heap::map_space()->Contains(obj)) {
     ASSERT(obj->IsMap());
-    live_map_objects_++;
+    live_map_objects_ += scale;
   } else if (Heap::old_pointer_space()->Contains(obj)) {
-    live_old_pointer_objects_++;
+    live_old_pointer_objects_ += scale;
   } else if (Heap::old_data_space()->Contains(obj)) {
-    live_old_data_objects_++;
+    live_old_data_objects_ += scale;
   } else if (Heap::code_space()->Contains(obj)) {
-    live_code_objects_++;
+    live_code_objects_ += scale;
   } else if (Heap::lo_space()->Contains(obj)) {
-    live_lo_objects_++;
+    live_lo_objects_ +=scale;
   } else {
     UNREACHABLE();
   }
 }
 #endif  // DEBUG
 
 
 void MarkCompactCollector::SweepLargeObjectSpace() {
 #ifdef DEBUG
   ASSERT(state_ == MARK_LIVE_OBJECTS);
@@ skipping 966 matching lines @@
 
 void MarkCompactCollector::RebuildRSets() {
 #ifdef DEBUG
   ASSERT(state_ == RELOCATE_OBJECTS);
   state_ = REBUILD_RSETS;
 #endif
   Heap::RebuildRSets();
 }
 
 } }  // namespace v8::internal