Revision: 24900
Author:   [email protected]
Date:     Mon Oct 27 11:57:58 2014 UTC
Log: Revert "Limit the number of transitions allowed per hidden class." Due to crashes in ClearMapTransitions

BUG=
[email protected]

Review URL: https://codereview.chromium.org/661583004
https://code.google.com/p/v8/source/detail?r=24900

Modified:
 /branches/bleeding_edge/src/heap/mark-compact.cc
 /branches/bleeding_edge/src/objects-inl.h
 /branches/bleeding_edge/src/objects.cc
 /branches/bleeding_edge/src/objects.h
 /branches/bleeding_edge/src/transitions-inl.h
 /branches/bleeding_edge/src/transitions.cc
 /branches/bleeding_edge/src/transitions.h
 /branches/bleeding_edge/test/cctest/test-heap.cc

=======================================
--- /branches/bleeding_edge/src/heap/mark-compact.cc Fri Oct 24 05:29:54 2014 UTC +++ /branches/bleeding_edge/src/heap/mark-compact.cc Mon Oct 27 11:57:58 2014 UTC
@@ -2521,14 +2521,13 @@

// Note that we never eliminate a transition array, though we might right-trim
   // such that number_of_transitions() == 0. If this assumption changes,
- // TransitionArray::Insert() will need to deal with the case that a transition
-  // array disappeared during GC.
-  int trim = t->number_of_transitions_storage() - transition_index;
+  // 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) {
     heap_->RightTrimFixedArray<Heap::FROM_GC>(
         t, t->IsSimpleTransition() ? trim
: trim * TransitionArray::kTransitionSize);
-    t->SetNumberOfTransitions(transition_index);
   }
   DCHECK(map->HasTransitionArray());
 }
=======================================
--- /branches/bleeding_edge/src/objects-inl.h   Fri Oct 24 05:29:54 2014 UTC
+++ /branches/bleeding_edge/src/objects-inl.h   Mon Oct 27 11:57:58 2014 UTC
@@ -5203,8 +5203,9 @@

 bool Map::CanHaveMoreTransitions() {
   if (!HasTransitionArray()) return true;
-  return transitions()->number_of_transitions() <=
-         TransitionArray::kMaxNumberOfTransitions;
+  return FixedArray::SizeFor(transitions()->length() +
+                             TransitionArray::kTransitionSize)
+      <= Page::kMaxRegularHeapObjectSize;
 }


@@ -6992,14 +6993,6 @@
   DCHECK(!heap->InNewSpace(heap->empty_fixed_array()));
   WRITE_FIELD(this, kCodeCacheOffset, heap->empty_fixed_array());
 }
-
-
-int Map::SlackForArraySize(int old_size, int size_limit) {
-  const int max_slack = size_limit - old_size;
-  DCHECK(max_slack >= 0);
-  if (old_size < 4) return Min(max_slack, 1);
-  return Min(max_slack, old_size / 2);
-}


 void JSArray::EnsureSize(Handle<JSArray> array, int required_size) {
=======================================
--- /branches/bleeding_edge/src/objects.cc      Fri Oct 24 18:04:03 2014 UTC
+++ /branches/bleeding_edge/src/objects.cc      Mon Oct 27 11:57:58 2014 UTC
@@ -6584,8 +6584,7 @@
     if (old_size == 0) {
       descriptors = DescriptorArray::Allocate(map->GetIsolate(), 0, 1);
     } else {
-      EnsureDescriptorSlack(
-          map, SlackForArraySize(old_size, kMaxNumberOfDescriptors));
+      EnsureDescriptorSlack(map, old_size < 4 ? 1 : old_size / 2);
       descriptors = handle(map->instance_descriptors());
     }
   }
@@ -6610,11 +6609,8 @@
     DCHECK(child->is_prototype_map());
   } else {
     Handle<TransitionArray> transitions =
-        TransitionArray::Insert(parent, name, child, flag);
-    if (!parent->HasTransitionArray() ||
-        *transitions != parent->transitions()) {
-      parent->set_transitions(*transitions);
-    }
+        TransitionArray::CopyInsert(parent, name, child, flag);
+    parent->set_transitions(*transitions);
     child->SetBackPointer(*parent);
   }
 }
=======================================
--- /branches/bleeding_edge/src/objects.h       Fri Oct 24 18:04:03 2014 UTC
+++ /branches/bleeding_edge/src/objects.h       Mon Oct 27 11:57:58 2014 UTC
@@ -6119,8 +6119,6 @@
   static void AppendCallbackDescriptors(Handle<Map> map,
                                         Handle<Object> descriptors);

-  static inline int SlackForArraySize(int old_size, int size_limit);
-
   static void EnsureDescriptorSlack(Handle<Map> map, int slack);

   // Returns the found code or undefined if absent.
=======================================
--- /branches/bleeding_edge/src/transitions-inl.h Fri Oct 24 05:29:54 2014 UTC +++ /branches/bleeding_edge/src/transitions-inl.h Mon Oct 27 11:57:58 2014 UTC
@@ -158,15 +158,6 @@
   FixedArray::NoIncrementalWriteBarrierSet(
       this, ToTargetIndex(transition_number), target);
 }
-
-
-void TransitionArray::SetNumberOfTransitions(int number_of_transitions) {
-  if (IsFullTransitionArray()) {
-    DCHECK(number_of_transitions <= number_of_transitions_storage());
-    WRITE_FIELD(this, kTransitionLengthOffset,
-                Smi::FromInt(number_of_transitions));
-  }
-}


 #undef FIELD_ADDR
=======================================
--- /branches/bleeding_edge/src/transitions.cc  Fri Oct 24 05:29:54 2014 UTC
+++ /branches/bleeding_edge/src/transitions.cc  Mon Oct 27 11:57:58 2014 UTC
@@ -13,12 +13,10 @@


 Handle<TransitionArray> TransitionArray::Allocate(Isolate* isolate,
- int number_of_transitions,
-                                                  int slack) {
-  Handle<FixedArray> array = isolate->factory()->NewFixedArray(
-      LengthFor(number_of_transitions + slack));
+ int number_of_transitions) {
+  Handle<FixedArray> array =
+      isolate->factory()->NewFixedArray(ToKeyIndex(number_of_transitions));
   array->set(kPrototypeTransitionsIndex, Smi::FromInt(0));
-  array->set(kTransitionLengthIndex, Smi::FromInt(number_of_transitions));
   return Handle<TransitionArray>::cast(array);
 }

@@ -76,7 +74,6 @@
   if (new_nof != nof) {
     DCHECK(new_nof == 0);
     result->Shrink(ToKeyIndex(0));
-    result->SetNumberOfTransitions(0);
   } else if (nof == 1) {
     result->NoIncrementalWriteBarrierCopyFrom(
         containing_map->transitions(), kSimpleTransitionIndex, 0);
@@ -88,47 +85,21 @@
 }


-Handle<TransitionArray> TransitionArray::Insert(Handle<Map> map,
-                                                Handle<Name> name,
-                                                Handle<Map> target,
- SimpleTransitionFlag flag) {
+Handle<TransitionArray> TransitionArray::CopyInsert(Handle<Map> map,
+                                                    Handle<Name> name,
+                                                    Handle<Map> target,
+ SimpleTransitionFlag flag) {
   if (!map->HasTransitionArray()) {
     return TransitionArray::NewWith(map, name, target, flag);
   }

   int number_of_transitions = map->transitions()->number_of_transitions();
-  int new_nof = number_of_transitions;
+  int new_size = number_of_transitions;

   int insertion_index = map->transitions()->Search(*name);
-  if (insertion_index == kNotFound) ++new_nof;
-  DCHECK(new_nof <= kMaxNumberOfTransitions);
+  if (insertion_index == kNotFound) ++new_size;

-  if (new_nof <= map->transitions()->number_of_transitions_storage()) {
-    DisallowHeapAllocation no_gc;
-    TransitionArray* array = map->transitions();
-
-    if (insertion_index != kNotFound) {
-      array->SetTarget(insertion_index, *target);
-      return handle(array);
-    }
-
-    array->SetNumberOfTransitions(new_nof);
-    uint32_t hash = name->Hash();
-    for (insertion_index = number_of_transitions; insertion_index > 0;
-         --insertion_index) {
-      Name* key = array->GetKey(insertion_index - 1);
-      if (key->Hash() <= hash) break;
-      array->SetKey(insertion_index, key);
- array->SetTarget(insertion_index, array->GetTarget(insertion_index - 1));
-    }
-    array->SetKey(insertion_index, *name);
-    array->SetTarget(insertion_index, *target);
-    return handle(array);
-  }
-
-  Handle<TransitionArray> result = Allocate(
-      map->GetIsolate(), new_nof,
- Map::SlackForArraySize(number_of_transitions, kMaxNumberOfTransitions));
+  Handle<TransitionArray> result = Allocate(map->GetIsolate(), new_size);

// 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
@@ -140,18 +111,28 @@
     DCHECK(array->number_of_transitions() < number_of_transitions);

     number_of_transitions = array->number_of_transitions();
-    new_nof = number_of_transitions;
+    new_size = number_of_transitions;

     insertion_index = array->Search(*name);
-    if (insertion_index == kNotFound) ++new_nof;
+    if (insertion_index == kNotFound) ++new_size;

-    result->Shrink(ToKeyIndex(new_nof));
-    result->SetNumberOfTransitions(new_nof);
+    result->Shrink(ToKeyIndex(new_size));
   }

   if (array->HasPrototypeTransitions()) {
     result->SetPrototypeTransitions(array->GetPrototypeTransitions());
   }
+
+  if (insertion_index != kNotFound) {
+    for (int i = 0; i < number_of_transitions; ++i) {
+      if (i != insertion_index) {
+        result->NoIncrementalWriteBarrierCopyFrom(array, i, i);
+      }
+    }
+    result->NoIncrementalWriteBarrierSet(insertion_index, *name, *target);
+    result->set_back_pointer_storage(array->back_pointer_storage());
+    return result;
+  }

   insertion_index = 0;
   for (; insertion_index < number_of_transitions; ++insertion_index) {
=======================================
--- /branches/bleeding_edge/src/transitions.h   Fri Oct 24 05:29:54 2014 UTC
+++ /branches/bleeding_edge/src/transitions.h   Mon Oct 27 11:57:58 2014 UTC
@@ -30,9 +30,8 @@
 // The full format is:
 // [0] Undefined or back pointer map
 // [1] Smi(0) or fixed array of prototype transitions
-// [2] Number of transitions
-// [3] First transition
-// [3 + number of transitions * kTransitionSize]: start of slack
+// [2] First transition
+// [length() - kTransitionSize] Last transition
 class TransitionArray: public FixedArray {
  public:
   // Accessors for fetching instance transition at transition number.
@@ -68,21 +67,10 @@
   // Returns the number of transitions in the array.
   int number_of_transitions() {
     if (IsSimpleTransition()) return 1;
-    if (length() <= kFirstIndex) return 0;
-    return Smi::cast(get(kTransitionLengthIndex))->value();
+    int len = length();
+    return len <= kFirstIndex ? 0 : (len - kFirstIndex) / kTransitionSize;
   }

-  int number_of_transitions_storage() {
-    if (IsSimpleTransition()) return 1;
-    if (length() <= kFirstIndex) return 0;
-    return (length() - kFirstIndex) / kTransitionSize;
-  }
-
-  int NumberOfSlackTransitions() {
-    return number_of_transitions_storage() - number_of_transitions();
-  }
-
-  inline void SetNumberOfTransitions(int number_of_transitions);
   inline int number_of_entries() { return number_of_transitions(); }

   // Creates a FullTransitionArray from a SimpleTransitionArray in
@@ -90,22 +78,21 @@
   static Handle<TransitionArray> ExtendToFullTransitionArray(
       Handle<Map> containing_map);

-  // Return a transition array, using the array from the owning map if it
-  // already has one (copying into a larger array if necessary), otherwise
-  // creating a new one according to flag.
+ // Create a transition array, copying from the owning map if it already has
+  // one, otherwise creating a new one according to flag.
   // TODO(verwaest): This should not cause an existing transition to be
   // overwritten.
-  static Handle<TransitionArray> Insert(Handle<Map> map, Handle<Name> name,
-                                        Handle<Map> target,
-                                        SimpleTransitionFlag flag);
+  static Handle<TransitionArray> CopyInsert(Handle<Map> map,
+                                            Handle<Name> name,
+                                            Handle<Map> target,
+                                            SimpleTransitionFlag flag);

   // Search a transition for a given property name.
   inline int Search(Name* name);

   // Allocates a TransitionArray.
-  static Handle<TransitionArray> Allocate(Isolate* isolate,
-                                          int number_of_transitions,
-                                          int slack = 0);
+  static Handle<TransitionArray> Allocate(
+      Isolate* isolate, int number_of_transitions);

   bool IsSimpleTransition() {
     return length() == kSimpleTransitionSize &&
@@ -133,8 +120,7 @@

   // Layout for full transition arrays.
   static const int kPrototypeTransitionsIndex = 1;
-  static const int kTransitionLengthIndex = 2;
-  static const int kFirstIndex = 3;
+  static const int kFirstIndex = 2;

   // Layout for simple transition arrays.
   static const int kSimpleTransitionTarget = 1;
@@ -147,8 +133,6 @@
   // Layout for the full transition array header.
static const int kPrototypeTransitionsOffset = kBackPointerStorageOffset +
                                                  kPointerSize;
-  static const int kTransitionLengthOffset =
-      kPrototypeTransitionsOffset + kPointerSize;

   // Layout of map transition entries in full transition arrays.
   static const int kTransitionKey = 0;
@@ -172,12 +156,6 @@
// The maximum number of transitions we want in a transition array (should
   // fit in a page).
   static const int kMaxNumberOfTransitions = 1024 + 512;
-
-  // Returns the fixed array length required to hold number_of_transitions
-  // transitions.
-  static int LengthFor(int number_of_transitions) {
-    return ToKeyIndex(number_of_transitions);
-  }

  private:
   // Conversion from transition number to array indices.
=======================================
--- /branches/bleeding_edge/test/cctest/test-heap.cc Fri Oct 24 05:29:54 2014 UTC +++ /branches/bleeding_edge/test/cctest/test-heap.cc Mon Oct 27 11:57:58 2014 UTC
@@ -2848,7 +2848,6 @@
              "root = new F");
   root = GetByName("root");
   AddPropertyTo(2, root, "funny");
-  CcTest::heap()->CollectGarbage(NEW_SPACE);

// Count number of live transitions after marking. Note that one transition
   // is left, because 'o' still holds an instance of one transition target.
@@ -2875,7 +2874,6 @@

   root = GetByName("root");
   AddPropertyTo(2, root, "funny");
-  CcTest::heap()->CollectGarbage(NEW_SPACE);

// Count number of live transitions after marking. Note that one transition
   // is left, because 'o' still holds an instance of one transition target.

--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
--- You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/d/optout.

Reply via email to