Some tests and simplified TransitionArray copying

Tests for verifying that we deal correctly with shrinking transition
arrays while allocating a copy of one.

Also, we can rely on a transition array only shrinking and not
disappearing during gc while copying one.

R=verwaest@chromium.org

Review URL: https://codereview.chromium.org/232883003

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@20710 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

diff --git a/src/objects.cc b/src/objects.cc
index aaa0320..37bb4f5 100644 (file)
--- a/src/objects.cc
+++ b/src/objects.cc
@@ -9474,11 +9474,16 @@ void Map::ClearNonLiveTransitions(Heap* heap) {
     }
   }
 
+  // Note that we never eliminate a transition array, though we might right-trim
+  // such that number_of_transitions() == 0. If this assumption changes,
+  // TransitionArray::CopyInsert() will need to deal with the case that a
+  // transition array disappeared during GC.
   int trim = t->number_of_transitions() - transition_index;
   if (trim > 0) {
     RightTrimFixedArray<Heap::FROM_GC>(heap, t, t->IsSimpleTransition()
         ? trim : trim * TransitionArray::kTransitionSize);
   }
+  ASSERT(HasTransitionArray());
 }
 
 

diff --git a/src/transitions.cc b/src/transitions.cc
index 33b2475..8c72c0d 100644 (file)
--- a/src/transitions.cc
+++ b/src/transitions.cc
@@ -124,26 +124,11 @@ Handle<TransitionArray> TransitionArray::CopyInsert(Handle<Map> map,
 
   Handle<TransitionArray> result = Allocate(map->GetIsolate(), new_size);
 
-  // The map's transition array may have disappeared or grown smaller during
-  // the allocation above as it was weakly traversed. Trim the result copy if
-  // needed, and recompute variables.
+  // The map's transition array may grown smaller during the allocation above as
+  // it was weakly traversed, though it is guaranteed not to disappear. Trim the
+  // result copy if needed, and recompute variables.
+  ASSERT(map->HasTransitionArray());
   DisallowHeapAllocation no_gc;
-  if (!map->HasTransitionArray()) {
-    if (flag == SIMPLE_TRANSITION) {
-      ASSERT(result->length() >= kSimpleTransitionSize);
-      result->Shrink(kSimpleTransitionSize);
-      result->set(kSimpleTransitionTarget, *target);
-    } else {
-      ASSERT(result->length() >= ToKeyIndex(1));
-      result->Shrink(ToKeyIndex(1));
-      result->set(kPrototypeTransitionsIndex, Smi::FromInt(0));
-      result->NoIncrementalWriteBarrierSet(0, *name, *target);
-    }
-    result->set_back_pointer_storage(map->GetBackPointer());
-
-    return result;
-  }
-
   TransitionArray* array = map->transitions();
   if (array->number_of_transitions() != number_of_transitions) {
     ASSERT(array->number_of_transitions() < number_of_transitions);

diff --git a/test/cctest/test-heap.cc b/test/cctest/test-heap.cc
index 83a95f7..307e407 100644 (file)
--- a/test/cctest/test-heap.cc
+++ b/test/cctest/test-heap.cc
@@ -2671,6 +2671,145 @@ TEST(Regress1465) {
 }
 
 
+#ifdef DEBUG
+static void AddTransitions(int transitions_count) {
+  AlwaysAllocateScope always_allocate(CcTest::i_isolate());
+  for (int i = 0; i < transitions_count; i++) {
+    EmbeddedVector<char, 64> buffer;
+    OS::SNPrintF(buffer, "var o = new Object; o.prop%d = %d;", i, i);
+    CompileRun(buffer.start());
+  }
+}
+
+
+static Handle<JSObject> GetByName(const char* name) {
+  return v8::Utils::OpenHandle(
+      *v8::Handle<v8::Object>::Cast(
+          CcTest::global()->Get(v8_str(name))));
+}
+
+
+static void AddPropertyTo(
+    int gc_count, Handle<JSObject> object, const char* property_name) {
+  Isolate* isolate = CcTest::i_isolate();
+  Factory* factory = isolate->factory();
+  Handle<String> prop_name = factory->InternalizeUtf8String(property_name);
+  Handle<Smi> twenty_three(Smi::FromInt(23), isolate);
+  i::FLAG_gc_interval = gc_count;
+  i::FLAG_gc_global = true;
+  CcTest::heap()->set_allocation_timeout(gc_count);
+  JSReceiver::SetProperty(
+      object, prop_name, twenty_three, NONE, SLOPPY).Check();
+}
+
+
+TEST(TransitionArrayShrinksDuringAllocToZero) {
+  i::FLAG_stress_compaction = false;
+  i::FLAG_allow_natives_syntax = true;
+  CcTest::InitializeVM();
+  v8::HandleScope scope(CcTest::isolate());
+  static const int transitions_count = 10;
+  AddTransitions(transitions_count);
+  CompileRun("var root = new Object;");
+  Handle<JSObject> root = GetByName("root");
+
+  // Count number of live transitions before marking.
+  int transitions_before = CountMapTransitions(root->map());
+  CHECK_EQ(transitions_count, transitions_before);
+
+  // Get rid of o
+  CompileRun("o = new Object;"
+             "root = new Object");
+  root = GetByName("root");
+  AddPropertyTo(2, root, "funny");
+
+  // Count number of live transitions after marking.  Note that one transition
+  // is left, because 'o' still holds an instance of one transition target.
+  int transitions_after = CountMapTransitions(
+      Map::cast(root->map()->GetBackPointer()));
+  CHECK_EQ(1, transitions_after);
+}
+
+
+TEST(TransitionArrayShrinksDuringAllocToOne) {
+  i::FLAG_stress_compaction = false;
+  i::FLAG_allow_natives_syntax = true;
+  CcTest::InitializeVM();
+  v8::HandleScope scope(CcTest::isolate());
+  static const int transitions_count = 10;
+  AddTransitions(transitions_count);
+  CompileRun("var root = new Object;");
+  Handle<JSObject> root = GetByName("root");
+
+  // Count number of live transitions before marking.
+  int transitions_before = CountMapTransitions(root->map());
+  CHECK_EQ(transitions_count, transitions_before);
+
+  root = GetByName("root");
+  AddPropertyTo(2, root, "funny");
+
+  // Count number of live transitions after marking.  Note that one transition
+  // is left, because 'o' still holds an instance of one transition target.
+  int transitions_after = CountMapTransitions(
+      Map::cast(root->map()->GetBackPointer()));
+  CHECK_EQ(2, transitions_after);
+}
+
+
+TEST(TransitionArrayShrinksDuringAllocToOnePropertyFound) {
+  i::FLAG_stress_compaction = false;
+  i::FLAG_allow_natives_syntax = true;
+  CcTest::InitializeVM();
+  v8::HandleScope scope(CcTest::isolate());
+  static const int transitions_count = 10;
+  AddTransitions(transitions_count);
+  CompileRun("var root = new Object;");
+  Handle<JSObject> root = GetByName("root");
+
+  // Count number of live transitions before marking.
+  int transitions_before = CountMapTransitions(root->map());
+  CHECK_EQ(transitions_count, transitions_before);
+
+  root = GetByName("root");
+  AddPropertyTo(0, root, "prop9");
+
+  // Count number of live transitions after marking.  Note that one transition
+  // is left, because 'o' still holds an instance of one transition target.
+  int transitions_after = CountMapTransitions(
+      Map::cast(root->map()->GetBackPointer()));
+  CHECK_EQ(1, transitions_after);
+}
+
+
+TEST(TransitionArraySimpleToFull) {
+  i::FLAG_stress_compaction = false;
+  i::FLAG_allow_natives_syntax = true;
+  CcTest::InitializeVM();
+  v8::HandleScope scope(CcTest::isolate());
+  static const int transitions_count = 1;
+  AddTransitions(transitions_count);
+  CompileRun("var root = new Object;");
+  Handle<JSObject> root = GetByName("root");
+
+  // Count number of live transitions before marking.
+  int transitions_before = CountMapTransitions(root->map());
+  CHECK_EQ(transitions_count, transitions_before);
+
+  CompileRun("o = new Object;"
+             "root = new Object");
+  root = GetByName("root");
+  ASSERT(root->map()->transitions()->IsSimpleTransition());
+  AddPropertyTo(2, root, "happy");
+
+  // Count number of live transitions after marking.  Note that one transition
+  // is left, because 'o' still holds an instance of one transition target.
+  int transitions_after = CountMapTransitions(
+      Map::cast(root->map()->GetBackPointer()));
+  CHECK_EQ(1, transitions_after);
+}
+#endif  // DEBUG
+
+
 TEST(Regress2143a) {
   i::FLAG_collect_maps = true;
   i::FLAG_incremental_marking = true;