Fixes a couple problems with GC of unoptimized code.

runtime/vm/pages.cc

 // Copyright (c) 2012, 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/pages.h"
 
 #include "platform/assert.h"
 #include "vm/compiler_stats.h"
 #include "vm/gc_marker.h"
 #include "vm/gc_sweeper.h"
@@ skipping 11 matching lines @@
 DEFINE_FLAG(bool, print_free_list_before_gc, false,
             "Print free list statistics before a GC");
 DEFINE_FLAG(bool, print_free_list_after_gc, false,
             "Print free list statistics after a GC");
 DEFINE_FLAG(bool, collect_code, false,
             "Attempt to GC infrequently used code.");
 DEFINE_FLAG(int, code_collection_interval_in_us, 30000000,
             "Time between attempts to collect unused code.");
 DEFINE_FLAG(bool, log_code_drop, false,
             "Emit a log message when pointers to unused code are dropped.");
+DEFINE_FLAG(bool, always_drop_code, false,
+            "Always try to drop code if the function's usage counter is > 0");
 
 HeapPage* HeapPage::Initialize(VirtualMemory* memory, PageType type) {
   ASSERT(memory->size() > VirtualMemory::PageSize());
   bool is_executable = (type == kExecutable);
   memory->Commit(is_executable);
 
   HeapPage* result = reinterpret_cast<HeapPage*>(memory->address());
   result->memory_ = memory;
   result->next_ = NULL;
   result->executable_ = is_executable;
@@ skipping 335 matching lines @@
     page = page->next();
   }
   page = large_pages_;
   while (page != NULL) {
     page->WriteProtect(read_only);
     page = page->next();
   }
 }
 
 
-class CodeDetacherVisitor : public ObjectVisitor {
- public:
-  explicit CodeDetacherVisitor(Isolate* isolate) : ObjectVisitor(isolate) { }
-
-  virtual void VisitObject(RawObject* obj);
-
- private:
-  static bool MayDetachCode(const Function& fn);
-  DISALLOW_COPY_AND_ASSIGN(CodeDetacherVisitor);
-};
-
-
-bool CodeDetacherVisitor::MayDetachCode(const Function& fn) {
-  return fn.HasCode() &&  // Not already detached.
-         !fn.HasOptimizedCode() &&
-         !fn.HasBreakpoint() &&
-         (fn.usage_counter() > 0);
-}
-
-
-void CodeDetacherVisitor::VisitObject(RawObject* raw_obj) {
-  Isolate* isolate = Isolate::Current();
-  HANDLESCOPE(isolate);
-  const Object& obj = Object::Handle(raw_obj);
-  if (obj.GetClassId() == kFunctionCid) {
-    const Function& fn = Function::Cast(obj);
-    if (CodeDetacherVisitor::MayDetachCode(fn)) {
-      fn.set_usage_counter(fn.usage_counter() / 2);
-      if (fn.usage_counter() == 0) {
-        if (FLAG_log_code_drop) {
-          const String& name = String::Handle(fn.name());
-          OS::Print("Detaching code for function %s\n", name.ToCString());
-        }
-        fn.DetachCode();
-      }
-    }
-  }
-}
-
-
-void PageSpace::TryDetachingCode() {
+bool PageSpace::ShouldTryCollectingCode() {
   // Try to collect code if enough time has passed since the last attempt.
   const int64_t start = OS::GetCurrentTimeMicros();
   const int64_t last_code_collection_in_us =
       page_space_controller_.last_code_collection_in_us();
+
   if ((start - last_code_collection_in_us) >
-       FLAG_code_collection_interval_in_us) {
+      FLAG_code_collection_interval_in_us) {
     if (FLAG_log_code_drop) {
       OS::Print("Trying to detach code.\n");
     }
-    Isolate* isolate = Isolate::Current();
-    CodeDetacherVisitor code_detacher(isolate);
-    heap_->IterateObjects(&code_detacher);
     page_space_controller_.set_last_code_collection_in_us(start);
+    return true;
   }
+  return false;
 }
 
 
 void PageSpace::MarkSweep(bool invoke_api_callbacks) {
   // MarkSweep is not reentrant. Make sure that is the case.
   ASSERT(!sweeping_);
   sweeping_ = true;
   Isolate* isolate = Isolate::Current();
-  if (FLAG_collect_code) {
-    TryDetachingCode();
-  }
 
   NoHandleScope no_handles(isolate);
 
   if (FLAG_print_free_list_before_gc) {
     OS::Print("Data Freelist (before GC):\n");
     freelist_[HeapPage::kData].Print();
     OS::Print("Executable Freelist (before GC):\n");
     freelist_[HeapPage::kExecutable].Print();
   }
 
   if (FLAG_verify_before_gc) {
     OS::PrintErr("Verifying before MarkSweep...");
     heap_->Verify();
     OS::PrintErr(" done.\n");
   }
 
   const int64_t start = OS::GetCurrentTimeMicros();
 
   // Mark all reachable old-gen objects.
+  bool try_collecting_code = FLAG_collect_code && ShouldTryCollectingCode();

[Ivan Posva, 2013/11/19 19:24:48]

collect_code and ShouldCollectCode() please.

[zra, 2013/11/22 17:18:54]

Done.

   GCMarker marker(heap_);
-  marker.MarkObjects(isolate, this, invoke_api_callbacks);
+  marker.MarkObjects(isolate, this, invoke_api_callbacks, try_collecting_code);
 
   int64_t mid1 = OS::GetCurrentTimeMicros();
 
   // Reset the bump allocation page to unused.
   // Reset the freelists and setup sweeping.
   freelist_[HeapPage::kData].Reset();
   freelist_[HeapPage::kExecutable].Reset();
 
   int64_t mid2 = OS::GetCurrentTimeMicros();
 
@@ skipping 171 matching lines @@
     return 0;
   } else {
     ASSERT(total_time >= gc_time);
     int result= static_cast<int>((static_cast<double>(gc_time) /
                              static_cast<double>(total_time)) * 100);
     return result;
   }
 }
 
 }  // namespace dart