Unify processing of weak lists in heap.cc

src/heap.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 1472 matching lines @@
   if (external_string_table_.old_space_strings_.length() > 0) {
     Object** start = &external_string_table_.old_space_strings_[0];
     Object** end = start + external_string_table_.old_space_strings_.length();
     for (Object** p = start; p < end; ++p) *p = updater_func(this, p);
   }
 
   UpdateNewSpaceReferencesInExternalStringTable(updater_func);
 }
 
 
-static Object* ProcessFunctionWeakReferences(Heap* heap,
-                                             Object* function,
-                                             WeakObjectRetainer* retainer,
-                                             bool record_slots) {
+template <class T>
+struct WeakListVisitor;
+
+
+template <class T>
+static Object* VisitWeakList(Object* list,
+                      Heap* heap,

[Michael Starzinger, 2013/06/11 12:59:53]

nit: Indentation is off, also put the "record_slots" parameter on its own line.

[Dmitry Lomov (no reviews), 2013/06/11 14:47:11]

Done.

+                      WeakObjectRetainer* retainer, bool record_slots) {
   Object* undefined = heap->undefined_value();
   Object* head = undefined;
-  JSFunction* tail = NULL;
-  Object* candidate = function;
-  while (candidate != undefined) {
+  T* tail = NULL;
+  MarkCompactCollector* collector = heap->mark_compact_collector();
+  while (list != undefined) {
     // Check whether to keep the candidate in the list.
-    JSFunction* candidate_function = reinterpret_cast<JSFunction*>(candidate);
-    Object* retain = retainer->RetainAs(candidate);
-    if (retain != NULL) {
+    T* candidate = reinterpret_cast<T*>(list);
+    Object* retained = retainer->RetainAs(list);
+    if (retained != NULL) {
       if (head == undefined) {
         // First element in the list.
-        head = retain;
+        head = retained;
       } else {
         // Subsequent elements in the list.
         ASSERT(tail != NULL);
-        tail->set_next_function_link(retain);
+        WeakListVisitor<T>::set_weak_next(heap, tail, retained);
         if (record_slots) {
-          Object** next_function =
-              HeapObject::RawField(tail, JSFunction::kNextFunctionLinkOffset);
-          heap->mark_compact_collector()->RecordSlot(
-              next_function, next_function, retain);
+          Object** next_slot =
+            HeapObject::RawField(tail, WeakListVisitor<T>::WeakNextOffset());
+          collector->RecordSlot(next_slot, next_slot, retained);
         }
       }
-      // Retained function is new tail.
-      candidate_function = reinterpret_cast<JSFunction*>(retain);
-      tail = candidate_function;
+      // Retained object is new tail.
+      ASSERT(!retained->IsUndefined());
+      candidate = reinterpret_cast<T*>(retained);
+      tail = candidate;
 
-      ASSERT(retain->IsUndefined() || retain->IsJSFunction());
 
-      if (retain == undefined) break;
+      // tail is a live object, visit it.
+      WeakListVisitor<T>::VisitLiveObject(
+          tail, heap, retainer, record_slots);
     }
 
     // Move to next element in the list.
-    candidate = candidate_function->next_function_link();
+    list = WeakListVisitor<T>::get_weak_next(candidate);
   }
 
   // Terminate the list if there is one or more elements.
   if (tail != NULL) {
-    tail->set_next_function_link(undefined);
+    WeakListVisitor<T>::set_weak_next(heap, tail, undefined);
+  }
+  return head;
+}
+
+
+template<>
+struct WeakListVisitor<JSFunction> {
+  static void set_weak_next(Heap*, JSFunction* function, Object* next) {
+    function->set_next_function_link(next);
   }
 
-  return head;
-}
+  static Object* get_weak_next(JSFunction* function) {
+    return function->next_function_link();
+  }
+
+  static int WeakNextOffset() {
+    return JSFunction::kNextFunctionLinkOffset;
+  }
+
+  static void VisitLiveObject(JSFunction*, Heap*,
+                              WeakObjectRetainer*, bool) {
+  }
+};
+
+
+template<>
+struct WeakListVisitor<Context> {
+  static void set_weak_next(Heap* heap, Context* context, Object* next) {
+    context->set_unchecked(heap,

[Michael Starzinger, 2013/06/11 12:59:53]

I think we can finally switch to use the checked accessor here, this means
'context' will always be a valid heap-object and we don't need to thread the
Heap pointer through set_weak_next.

This is a leftover from the old GC implementation that used in-place marking and
destroyed the map word in object headers. It is tracked by issue 1490.

[Dmitry Lomov (no reviews), 2013/06/11 14:47:11]

Done.

+                           Context::NEXT_CONTEXT_LINK,
+                           next,
+                           UPDATE_WRITE_BARRIER);
+  }
+
+  static Object* get_weak_next(Context* context) {
+    return context->get(Context::NEXT_CONTEXT_LINK);
+  }
+
+  static void VisitLiveObject(Context* context,
+                              Heap* heap,
+                              WeakObjectRetainer* retainer,
+                              bool record_slots) {
+    // Process the weak list of optimized functions for the context.
+    Object* function_list_head =
+        VisitWeakList<JSFunction>(
+            context->get(Context::OPTIMIZED_FUNCTIONS_LIST),
+            heap,
+            retainer,
+            record_slots);
+    context->set_unchecked(heap,
+                           Context::OPTIMIZED_FUNCTIONS_LIST,
+                           function_list_head,
+                           UPDATE_WRITE_BARRIER);
+    if (record_slots) {
+      Object** optimized_functions =
+          HeapObject::RawField(
+              context, FixedArray::SizeFor(Context::OPTIMIZED_FUNCTIONS_LIST));
+      heap->mark_compact_collector()->RecordSlot(
+          optimized_functions, optimized_functions, function_list_head);
+    }
+  }
+
+  static int WeakNextOffset() {
+    return FixedArray::SizeFor(Context::NEXT_CONTEXT_LINK);
+  }
+};
 
 
 void Heap::ProcessWeakReferences(WeakObjectRetainer* retainer) {
   // We don't record weak slots during marking or scavenges.
   // Instead we do it once when we complete mark-compact cycle.
   // Note that write barrier has no effect if we are already in the middle of
   // compacting mark-sweep cycle and we have to record slots manually.
   bool record_slots =
       gc_state() == MARK_COMPACT &&
       mark_compact_collector()->is_compacting();
   ProcessArrayBuffers(retainer, record_slots);
   ProcessNativeContexts(retainer, record_slots);
 }
 
 void Heap::ProcessNativeContexts(WeakObjectRetainer* retainer,
                                  bool record_slots) {
-  Object* undefined = undefined_value();
-  Object* head = undefined;
-  Context* tail = NULL;
-  Object* candidate = native_contexts_list_;
-
-  while (candidate != undefined) {
-    // Check whether to keep the candidate in the list.
-    Context* candidate_context = reinterpret_cast<Context*>(candidate);
-    Object* retain = retainer->RetainAs(candidate);
-    if (retain != NULL) {
-      if (head == undefined) {
-        // First element in the list.
-        head = retain;
-      } else {
-        // Subsequent elements in the list.
-        ASSERT(tail != NULL);
-        tail->set_unchecked(this,
-                            Context::NEXT_CONTEXT_LINK,
-                            retain,
-                            UPDATE_WRITE_BARRIER);
-
-        if (record_slots) {
-          Object** next_context =
-              HeapObject::RawField(
-                  tail, FixedArray::SizeFor(Context::NEXT_CONTEXT_LINK));
-          mark_compact_collector()->RecordSlot(
-              next_context, next_context, retain);
-        }
-      }
-      // Retained context is new tail.
-      candidate_context = reinterpret_cast<Context*>(retain);
-      tail = candidate_context;
-
-      if (retain == undefined) break;
-
-      // Process the weak list of optimized functions for the context.
-      Object* function_list_head =
-          ProcessFunctionWeakReferences(
-              this,
-              candidate_context->get(Context::OPTIMIZED_FUNCTIONS_LIST),
-              retainer,
-              record_slots);
-      candidate_context->set_unchecked(this,
-                                       Context::OPTIMIZED_FUNCTIONS_LIST,
-                                       function_list_head,
-                                       UPDATE_WRITE_BARRIER);
-      if (record_slots) {
-        Object** optimized_functions =
-            HeapObject::RawField(
-                tail, FixedArray::SizeFor(Context::OPTIMIZED_FUNCTIONS_LIST));
-        mark_compact_collector()->RecordSlot(
-            optimized_functions, optimized_functions, function_list_head);
-      }
-    }
-
-    // Move to next element in the list.
-    candidate = candidate_context->get(Context::NEXT_CONTEXT_LINK);
-  }
-
-  // Terminate the list if there is one or more elements.
-  if (tail != NULL) {
-    tail->set_unchecked(this,
-                        Context::NEXT_CONTEXT_LINK,
-                        Heap::undefined_value(),
-                        UPDATE_WRITE_BARRIER);
-  }
-
+  Object* head =
+      VisitWeakList<Context>(
+          native_contexts_list(), this, retainer, record_slots);
   // Update the head of the list of contexts.
   native_contexts_list_ = head;
 }
 
 
-template <class T>
-struct WeakListVisitor;
-
-
-template <class T>
-static Object* VisitWeakList(Object* list,
-                      MarkCompactCollector* collector,
-                      WeakObjectRetainer* retainer, bool record_slots) {
-  Object* head = Smi::FromInt(0);
-  T* tail = NULL;
-  while (list != Smi::FromInt(0)) {
-    Object* retained = retainer->RetainAs(list);
-    if (retained != NULL) {
-      if (head == Smi::FromInt(0)) {
-        head = retained;
-      } else {
-        ASSERT(tail != NULL);
-        WeakListVisitor<T>::set_weak_next(tail, retained);
-        if (record_slots) {
-          Object** next_slot =
-            HeapObject::RawField(tail, WeakListVisitor<T>::kWeakNextOffset);
-          collector->RecordSlot(next_slot, next_slot, retained);
-        }
-      }
-      tail = reinterpret_cast<T*>(retained);
-      WeakListVisitor<T>::VisitLiveObject(
-          tail, collector, retainer, record_slots);
-    }
-    list = WeakListVisitor<T>::get_weak_next(reinterpret_cast<T*>(list));
-  }
-  if (tail != NULL) {
-    tail->set_weak_next(Smi::FromInt(0));
-  }
-  return head;
-}
-
-
 template<>
 struct WeakListVisitor<JSTypedArray> {
-  static void set_weak_next(JSTypedArray* obj, Object* next) {
+  static void set_weak_next(Heap*, JSTypedArray* obj, Object* next) {

[Michael Starzinger, 2013/06/11 12:59:53]

See my previous comment, I think we can drop the Heap pointer argument here and
for the other visitors.

[Dmitry Lomov (no reviews), 2013/06/11 14:47:11]

Done.

     obj->set_weak_next(next);
   }
 
   static Object* get_weak_next(JSTypedArray* obj) {
     return obj->weak_next();
   }
 
   static void VisitLiveObject(JSTypedArray* obj,
-                              MarkCompactCollector* collector,
+                              Heap* heap,

[Michael Starzinger, 2013/06/11 12:59:53]

nit: Can we make the Heap pointer be the first argument to the function? This
would be in line with the other visitors in the code-base.

[Dmitry Lomov (no reviews), 2013/06/11 14:47:11]

Done.

                               WeakObjectRetainer* retainer,
                               bool record_slots) {}
 
-  static const int kWeakNextOffset = JSTypedArray::kWeakNextOffset;
+  static int WeakNextOffset() {
+    return JSTypedArray::kWeakNextOffset;
+  }
 };
 
 
 template<>
 struct WeakListVisitor<JSArrayBuffer> {
-  static void set_weak_next(JSArrayBuffer* obj, Object* next) {
+  static void set_weak_next(Heap*, JSArrayBuffer* obj, Object* next) {
     obj->set_weak_next(next);
   }
 
   static Object* get_weak_next(JSArrayBuffer* obj) {
     return obj->weak_next();
   }
 
   static void VisitLiveObject(JSArrayBuffer* array_buffer,
-                              MarkCompactCollector* collector,
+                              Heap* heap,
                               WeakObjectRetainer* retainer,
                               bool record_slots) {
     Object* typed_array_obj =
         VisitWeakList<JSTypedArray>(array_buffer->weak_first_array(),
-                                    collector, retainer, record_slots);
+                                    heap, retainer, record_slots);
     array_buffer->set_weak_first_array(typed_array_obj);
-    if (typed_array_obj != Smi::FromInt(0) && record_slots) {
+    if (typed_array_obj != heap->undefined_value() && record_slots) {
       Object** slot = HeapObject::RawField(
           array_buffer, JSArrayBuffer::kWeakFirstArrayOffset);
-      collector->RecordSlot(slot, slot, typed_array_obj);
+      heap->mark_compact_collector()->RecordSlot(slot, slot, typed_array_obj);
     }
   }
 
-  static const int kWeakNextOffset = JSArrayBuffer::kWeakNextOffset;
+  static int WeakNextOffset() {
+    return JSArrayBuffer::kWeakNextOffset;
+  }
 };
 
 
 void Heap::ProcessArrayBuffers(WeakObjectRetainer* retainer,
                                bool record_slots) {
   Object* array_buffer_obj =
       VisitWeakList<JSArrayBuffer>(array_buffers_list(),
-                                   mark_compact_collector(),
+                                   this,
                                    retainer, record_slots);
   set_array_buffers_list(array_buffer_obj);
 }
 
 
 void Heap::VisitExternalResources(v8::ExternalResourceVisitor* visitor) {
   DisallowHeapAllocation no_allocation;
 
   // Both the external string table and the string table may contain
   // external strings, but neither lists them exhaustively, nor is the
@@ skipping 5050 matching lines @@
 
 bool Heap::CreateHeapObjects() {
   // Create initial maps.
   if (!CreateInitialMaps()) return false;
   if (!CreateApiObjects()) return false;
 
   // Create initial objects
   if (!CreateInitialObjects()) return false;
 
   native_contexts_list_ = undefined_value();
+  array_buffers_list_ = undefined_value();
   return true;
 }
 
 
 void Heap::SetStackLimits() {
   ASSERT(isolate_ != NULL);
   ASSERT(isolate_ == isolate());
   // On 64 bit machines, pointers are generally out of range of Smis.  We write
   // something that looks like an out of range Smi to the GC.
 
@@ skipping 1216 matching lines @@
   if (FLAG_parallel_recompilation) {
     heap_->relocation_mutex_->Lock();
 #ifdef DEBUG
     heap_->relocation_mutex_locked_by_optimizer_thread_ =
         heap_->isolate()->optimizing_compiler_thread()->IsOptimizerThread();
 #endif  // DEBUG
   }
 }
 
 } }  // namespace v8::internal