Before write-protecting a PageSpace, return bump allocation block to freelist.

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 486 matching lines @@
   HeapPage* page_;
 };
 
 
 void PageSpace::MakeIterable() const {
   // Assert not called from concurrent sweeper task.
   // TODO(koda): Use thread/task identity when implemented.
   ASSERT(Isolate::Current()->heap() != NULL);
   if (bump_top_ < bump_end_) {
     FreeListElement::AsElement(bump_top_, bump_end_ - bump_top_);
   }

[siva, 2015/04/15 16:11:56]

Is it ok to still have bump_top_ and bump_end_ point to an element in the
freelist here? I am wondering if bump_top_ and bump_end_ need to be set to 0
here?

[koda, 2015/04/15 16:24:06]

The call only writes a header to the block, but does not actually add it to any
freelist (FreeList::Free does). Writing the header ensures that we can walk the
entire space. We do this lazily to save a couple of writes in bump pointer
allocation.

 }
 
 
+void PageSpace::AbandonBumpAllocation() {
+  if (bump_top_ < bump_end_) {
+    freelist_[HeapPage::kData].Free(bump_top_, bump_end_ - bump_top_);
+    bump_top_ = 0;
+    bump_end_ = 0;
+  }
+}
+
+
 bool PageSpace::Contains(uword addr) const {
   for (ExclusivePageIterator it(this); !it.Done(); it.Advance()) {
     if (it.page()->Contains(addr)) {
       return true;
     }
   }
   return false;
 }
 
 
@@ skipping 77 matching lines @@
       if (obj != Object::null()) {
         return obj;
       }
     }
   }
   return Object::null();
 }
 
 
 void PageSpace::WriteProtect(bool read_only) {
+  if (read_only) {
+    // Avoid MakeIterable trying to write to the heap.
+    AbandonBumpAllocation();
+  }
   for (ExclusivePageIterator it(this); !it.Done(); it.Advance()) {
     it.page()->WriteProtect(read_only);
   }
 }
 
 
 void PageSpace::PrintToJSONObject(JSONObject* object) {
   Isolate* isolate = Isolate::Current();
   ASSERT(isolate != NULL);
   JSONObject space(object, "old");
@@ skipping 155 matching lines @@
 
   // Mark all reachable old-gen objects.
   bool collect_code = FLAG_collect_code && ShouldCollectCode();
   GCMarker marker(heap_);
   marker.MarkObjects(isolate, this, invoke_api_callbacks, collect_code);
   usage_.used_in_words = marker.marked_words();
 
   int64_t mid1 = OS::GetCurrentTimeMicros();
 
   // Abandon the remainder of the bump allocation block.
-  MakeIterable();
-  bump_top_ = 0;
-  bump_end_ = 0;
+  AbandonBumpAllocation();
   // Reset the freelists and setup sweeping.
   freelist_[HeapPage::kData].Reset();
   freelist_[HeapPage::kExecutable].Reset();
 
   int64_t mid2 = OS::GetCurrentTimeMicros();
   int64_t mid3 = 0;
 
   {
     if (FLAG_verify_before_gc) {
       OS::PrintErr("Verifying before sweeping...");
@@ skipping 321 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