Retain maps for several garbage collections

src/heap/mark-compact.cc

Index: src/heap/mark-compact.cc
diff --git a/src/heap/mark-compact.cc b/src/heap/mark-compact.cc
index 9f962f15eb706e42b8509c7df09b86bfd1c45adc..161c9eede5876309a5498b350b483f88ddc70c25 100644
--- a/src/heap/mark-compact.cc
+++ b/src/heap/mark-compact.cc
@@ -2123,6 +2123,50 @@ void MarkCompactCollector::ProcessTopOptimizedFrame(ObjectVisitor* visitor) {
 }
 
 
+void MarkCompactCollector::RetainMaps(ObjectVisitor* visitor) {
+  if (reduce_memory_footprint_ || abort_incremental_marking_ ||

[Hannes Payer (out of office), 2014/12/16 17:15:35]

Why do you bail out for abort_incremental_marking_?

[ulan, 2014/12/17 11:13:34]

The incremental marking is aborted for GC requested explicitly by tests and
dev-tools. In that cases we want to be precise. I added a comment in the code.

+      FLAG_retain_maps_for_n_gc == 0) {
+    return;
+  }
+
+  HeapObjectIterator map_iterator(heap()->map_space());
+  // The retaining counter goes from Map::kRetainingCounterStart
+  // down to Map::kRetainingCounterStart. This range can be narrowed

[Erik Corry, 2014/12/16 16:16:32]

Comment looks wrong.

[ulan, 2014/12/16 16:35:23]

Thanks, that should be "down to Map::kRetainingCounterEnd", will fix.

[ulan, 2014/12/17 11:13:34]

Done.

+  // by the FLAG_retain_maps_for_n_gc flag.
+  int retaining_counter_end =
+      Max(Map::kRetainingCounterEnd,
+          Map::kRetainingCounterStart - FLAG_retain_maps_for_n_gc);
+  for (HeapObject* obj = map_iterator.Next(); obj != NULL;
+       obj = map_iterator.Next()) {
+    Map* map = Map::cast(obj);
+    MarkBit map_mark = Marking::MarkBitFrom(map);
+    int counter = map->counter();
+    if (!map_mark.Get()) {
+      if (counter > Map::kRetainingCounterStart ||
+          counter <= retaining_counter_end) {
+        // The counter is outside of retaining range. Do not retain this map.
+        continue;
+      }
+      HeapObject* constructor = HeapObject::cast(map->constructor());
+      if (!Marking::MarkBitFrom(constructor).Get()) {
+        // The constructor is dead, no new objects with this map can
+        // be created. Do not retain this map.
+        continue;
+      }
+      map->set_counter(counter - 1);
+      SetMark(map, map_mark);
+      MarkCompactMarkingVisitor::IterateBody(map->map(), map);
+      ProcessMarkingDeque();

[Erik Corry, 2014/12/16 16:52:48]

Can't we just process the marking deque at the end, instead of inside the loop?
By doing it here you may mark maps live that you haven't got to yet, and they
won't age, even though they are only reachable through maps that are ageing.

[ulan, 2014/12/16 16:54:36]

Good point, I will move it to the end.

[ulan, 2014/12/17 11:13:34]

Done.

+    } else {
+      if (counter < Map::kRetainingCounterStart) {

[Hannes Payer (out of office), 2014/12/16 17:15:35]

else if

[ulan, 2014/12/17 11:13:34]

Done.

+        // Reset the counter for live maps.
+        map->set_counter(Map::kRetainingCounterStart);
+      }
+    }
+  }
+}
+
+
 void MarkCompactCollector::EnsureMarkingDequeIsCommittedAndInitialize() {
   if (marking_deque_memory_ == NULL) {
     marking_deque_memory_ = new base::VirtualMemory(4 * MB);
@@ -2222,6 +2266,8 @@ void MarkCompactCollector::MarkLiveObjects() {
 
   ProcessTopOptimizedFrame(&root_visitor);
 
+  RetainMaps(&root_visitor);

[Erik Corry, 2014/12/16 16:16:33]

This should be after the ephemeral stuff.  Maps that are kept alive by
ephemerally-alive objects should not age.

But even better would be to put it inside ClearNonLiveReferences. That already
iterates over the map space.  It is better for the cache not to iterate multiple
times.

[ulan, 2014/12/16 16:35:23]

It should not be after ephemeral marking, because a resurrected map could mark
the key of an ephemeron, which would require marking the value of the ephemeron.

The most precise would be to put RetainMaps inside the loop in
ProcessEphemeralMarking, but that would be prohibitively expensive. So I had to
use imprecise (some live maps will age) but faster approach.

[Erik Corry, 2014/12/16 16:52:48]

OK that makes sense in a sort of sad way.

[Hannes Payer (out of office), 2014/12/16 17:15:35]

Please write a comment about that somewhere.

[ulan, 2014/12/17 11:13:34]

Done.

+
   {
     GCTracer::Scope gc_scope(heap()->tracer(), GCTracer::Scope::MC_WEAKCLOSURE);