Refactor skipped code functions interface.

runtime/vm/gc_marker.cc

 // Copyright (c) 2011, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 #include "vm/gc_marker.h"
 
 #include <map>
 #include <utility>
 #include <vector>
 
@@ skipping 56 matching lines @@
       it->second->VisitPointers(visitor);
     }
   }
 
  private:
   Map delay_set_;
   Mutex* mutex_;
 };
 
 
+class SkippedCodeFunctions {
+ public:
+  SkippedCodeFunctions() {}
+
+  void Add(RawFunction* func) {
+    skipped_code_functions_.Add(func);
+  }
+
+  void DetachCode() {
+    intptr_t unoptimized_code_count = 0;
+    intptr_t current_code_count = 0;
+    for (int i = 0; i < skipped_code_functions_.length(); i++) {
+      RawFunction* func = skipped_code_functions_[i];
+      RawCode* code = func->ptr()->instructions_->ptr()->code_;
+      if (!code->IsMarked()) {
+        // If the code wasn't strongly visited through other references
+        // after skipping the function's code pointer, then we disconnect the
+        // code from the function.
+        func->StorePointer(
+            &(func->ptr()->instructions_),
+            StubCode::LazyCompile_entry()->code()->ptr()->instructions_);
+        if (FLAG_log_code_drop) {
+          // NOTE: This code runs while GC is in progress and runs within
+          // a NoHandleScope block. Hence it is not okay to use a regular Zone
+          // or Scope handle. We use a direct stack handle so the raw pointer in
+          // this handle is not traversed. The use of a handle is mainly to
+          // be able to reuse the handle based code and avoid having to add
+          // helper functions to the raw object interface.
+          String name;
+          name = func->ptr()->name_;
+          ISL_Print("Detaching code: %s\n", name.ToCString());
+          current_code_count++;
+        }
+      }
+
+      code = func->ptr()->unoptimized_code_;
+      if (!code->IsMarked()) {
+        // If the code wasn't strongly visited through other references
+        // after skipping the function's code pointer, then we disconnect the
+        // code from the function.
+        func->StorePointer(&(func->ptr()->unoptimized_code_), Code::null());
+        if (FLAG_log_code_drop) {
+          unoptimized_code_count++;
+        }
+      }
+    }
+    if (FLAG_log_code_drop) {
+      ISL_Print("  total detached current: %" Pd "\n", current_code_count);
+      ISL_Print("  total detached unoptimized: %" Pd "\n",
+                unoptimized_code_count);
+    }
+    // Clean up.
+    skipped_code_functions_.Clear();
+  }
+
+ private:
+  GrowableArray<RawFunction*> skipped_code_functions_;

[Ivan Posva, 2015/08/27 16:41:30]

DISALLOW*

[koda, 2015/08/27 17:51:32]

Done.

+};
+
+
 class MarkingVisitor : public ObjectPointerVisitor {
  public:
   MarkingVisitor(Isolate* isolate,
                  Heap* heap,
                  PageSpace* page_space,
                  MarkingStack* marking_stack,
                  DelaySet* delay_set,
-                 bool visit_function_code)
+                 SkippedCodeFunctions* skipped_code_functions)
       : ObjectPointerVisitor(isolate),
         thread_(Thread::Current()),
         heap_(heap),
         vm_heap_(Dart::vm_isolate()->heap()),
         class_table_(isolate->class_table()),
         page_space_(page_space),
         work_list_(marking_stack),
         delay_set_(delay_set),
         visiting_old_object_(NULL),
-        visit_function_code_(visit_function_code),
+        skipped_code_functions_(skipped_code_functions),
         marked_bytes_(0) {
     ASSERT(heap_ != vm_heap_);
     ASSERT(thread_->isolate() == isolate);
   }
 
   uintptr_t marked_bytes() const { return marked_bytes_; }
 
   // Returns true if some non-zero amount of work was performed.
   bool DrainMarkingStack() {
     RawObject* raw_obj = work_list_.Pop();
@@ skipping 19 matching lines @@
     VisitingOldObject(NULL);
     return true;
   }
 
   void VisitPointers(RawObject** first, RawObject** last) {
     for (RawObject** current = first; current <= last; current++) {
       MarkObject(*current, current);
     }
   }
 
-  bool visit_function_code() const { return visit_function_code_; }
+  bool visit_function_code() const {
+    return skipped_code_functions_ == NULL;
+  }
 
-  virtual MallocGrowableArray<RawFunction*>* skipped_code_functions() {
-    return &skipped_code_functions_;
+  virtual void add_skipped_code_function(RawFunction* func) {
+    ASSERT(!visit_function_code());
+    isolate()->counters()->Increment("skip", 1);
+    skipped_code_functions_->Add(func);
   }
 
   // Returns the mark bit. Sets the watch bit if unmarked. (The prior value of
   // the watched bit is returned in 'watched_before' for validation purposes.)
   // TODO(koda): When synchronizing header bits, this goes in a single CAS loop.
   static bool EnsureWatchedIfWhite(RawObject* obj, bool* watched_before) {
     if (obj->IsMarked()) {
       return false;
     }
     if (!obj->IsWatched()) {
@@ skipping 16 matching lines @@
       bool new_key = delay_set_->Insert(raw_weak);
       ASSERT(new_key == !watched_before);
     } else {
       // Key is gray or black.  Make the weak property black.
       raw_weak->VisitPointers(this);
     }
   }
 
   // Called when all marking is complete.
   void Finalize() {
-    if (!visit_function_code_) {

[Ivan Posva, 2015/08/27 16:41:30]

Why was this taken out of the marker? Shouldn't this be done in the different
marking threads for their collected code functions once all markers have
finished.

[koda, 2015/08/27 17:51:31]

Moved back, as discussed offline.

-      DetachCode();
-    }
     work_list_.Finalize();
   }
 
   void VisitingOldObject(RawObject* obj) {
     ASSERT((obj == NULL) || obj->IsOldObject());
     visiting_old_object_ = obj;
   }
 
  private:
   class WorkList : public ValueObject {
@@ skipping 89 matching lines @@
     }
     if (RawObject::IsVariableSizeClassId(raw_obj->GetClassId())) {
       class_table_->UpdateLiveOld(raw_obj->GetClassId(), raw_obj->Size());
     } else {
       class_table_->UpdateLiveOld(raw_obj->GetClassId(), 0);
     }
 
     MarkAndPush(raw_obj);
   }
 
-  void DetachCode() {
-    intptr_t unoptimized_code_count = 0;
-    intptr_t current_code_count = 0;
-    for (int i = 0; i < skipped_code_functions_.length(); i++) {
-      RawFunction* func = skipped_code_functions_[i];
-      RawCode* code = func->ptr()->instructions_->ptr()->code_;
-      if (!code->IsMarked()) {
-        // If the code wasn't strongly visited through other references
-        // after skipping the function's code pointer, then we disconnect the
-        // code from the function.
-        func->StorePointer(
-            &(func->ptr()->instructions_),
-            StubCode::LazyCompile_entry()->code()->ptr()->instructions_);
-        if (FLAG_log_code_drop) {
-          // NOTE: This code runs while GC is in progress and runs within
-          // a NoHandleScope block. Hence it is not okay to use a regular Zone
-          // or Scope handle. We use a direct stack handle so the raw pointer in
-          // this handle is not traversed. The use of a handle is mainly to
-          // be able to reuse the handle based code and avoid having to add
-          // helper functions to the raw object interface.
-          String name;
-          name = func->ptr()->name_;
-          ISL_Print("Detaching code: %s\n", name.ToCString());
-          current_code_count++;
-        }
-      }
-
-      code = func->ptr()->unoptimized_code_;
-      if (!code->IsMarked()) {
-        // If the code wasn't strongly visited through other references
-        // after skipping the function's code pointer, then we disconnect the
-        // code from the function.
-        func->StorePointer(&(func->ptr()->unoptimized_code_), Code::null());
-        if (FLAG_log_code_drop) {
-          unoptimized_code_count++;
-        }
-      }
-    }
-    if (FLAG_log_code_drop) {
-      ISL_Print("  total detached current: %" Pd "\n", current_code_count);
-      ISL_Print("  total detached unoptimized: %" Pd "\n",
-                unoptimized_code_count);
-    }
-    // Clean up.
-    skipped_code_functions_.Clear();
-  }
-
   Thread* thread_;
   Heap* heap_;
   Heap* vm_heap_;
   ClassTable* class_table_;
   PageSpace* page_space_;
   WorkList work_list_;
   DelaySet* delay_set_;
   RawObject* visiting_old_object_;
-  const bool visit_function_code_;
-  MallocGrowableArray<RawFunction*> skipped_code_functions_;
+  SkippedCodeFunctions* skipped_code_functions_;
   uintptr_t marked_bytes_;
 
   DISALLOW_IMPLICIT_CONSTRUCTORS(MarkingVisitor);
 };
 
 
 static bool IsUnreachable(const RawObject* raw_obj) {
   if (!raw_obj->IsHeapObject()) {
     return false;
   }
@@ skipping 166 matching lines @@
   ObjectIdRing* ring = isolate->object_id_ring();
   ASSERT(ring != NULL);
   ring->VisitPointers(&visitor);
 }
 
 
 void GCMarker::MarkObjects(Isolate* isolate,
                            PageSpace* page_space,
                            bool invoke_api_callbacks,
                            bool collect_code) {
-  const bool visit_function_code = !collect_code;
   Prologue(isolate, invoke_api_callbacks);
   // The API prologue/epilogue may create/destroy zones, so we must not
   // depend on zone allocations surviving beyond the epilogue callback.
   {
     StackZone zone(isolate);
     MarkingStack marking_stack;
     DelaySet delay_set;
+    SkippedCodeFunctions* skipped_code_functions =
+        collect_code ? new SkippedCodeFunctions() : NULL;

[Ivan Posva, 2015/08/27 16:41:30]

Why is this not stack or zone allocated?

[koda, 2015/08/27 17:51:32]

Done.

     MarkingVisitor mark(isolate, heap_, page_space, &marking_stack,
-                        &delay_set, visit_function_code);
+                        &delay_set, skipped_code_functions);
     IterateRoots(isolate, &mark, !invoke_api_callbacks);
     mark.DrainMarkingStack();
     IterateWeakReferences(isolate, &mark);
     MarkingWeakVisitor mark_weak;
     IterateWeakRoots(isolate, &mark_weak, invoke_api_callbacks);
-    // TODO(koda): Add hand-over callback and centralize skipped code functions.
+    // TODO(koda): Add hand-over callback.
     marked_bytes_ = mark.marked_bytes();
     mark.Finalize();
+    if (collect_code) {

[Ivan Posva, 2015/08/27 16:41:30]

if (skipped_code_functions != null) {

To protect against a change in the trigger of allocating this object.

[koda, 2015/08/27 17:51:31]

Done.

+      skipped_code_functions->DetachCode();
+      delete skipped_code_functions;
+    }
     delay_set.ClearReferences();
     ProcessWeakTables(page_space);
     ProcessObjectIdTable(isolate);
   }
   Epilogue(isolate, invoke_api_callbacks);
 }
 
 }  // namespace dart