Adding fast path for generalizing maps.

src/objects.cc

Index: src/objects.cc
diff --git a/src/objects.cc b/src/objects.cc
index 94fd487aa3eef6f6016bcb153e4c69ba2e608273..b378084e67dbf7bb9f53ade9a2bb4b02394f5a14 100644
--- a/src/objects.cc
+++ b/src/objects.cc
@@ -2460,12 +2460,17 @@ MaybeObject* Map::GeneralizeRepresentation(int modify_index,
   // Check the state of the root map.
   DescriptorArray* updated_descriptors = updated->instance_descriptors();
 
+  int valid = updated->NumberOfOwnDescriptors();
+  if (updated_descriptors->IsMoreGeneralThan(
+          verbatim, valid, descriptors, old_descriptors)) {
+    Representation updated_representation =
+        updated_descriptors->GetDetails(modify_index).representation();
+    if (new_representation.fits(updated_representation)) return updated;
+  }
+
   DescriptorArray* new_descriptors;
   MaybeObject* maybe_descriptors = updated_descriptors->Merge(
-      verbatim,
-      updated->NumberOfOwnDescriptors(),
-      descriptors,
-      old_descriptors);
+      verbatim, valid, descriptors, old_descriptors);
   if (!maybe_descriptors->To(&new_descriptors)) return maybe_descriptors;
 
   old_reprepresentation =
@@ -2477,9 +2482,9 @@ MaybeObject* Map::GeneralizeRepresentation(int modify_index,
       verbatim, descriptors, new_descriptors);
 
   int split_descriptors = split_map->NumberOfOwnDescriptors();
-  // Check whether |split_map| matches what we were looking for. If so, return
-  // it.
-  if (descriptors == split_descriptors) return split_map;
+  // This is shadowed by |updated_descriptors| being more general than
+  // |old_descriptors|.
+  ASSERT(descriptors != split_descriptors);
 
   int descriptor = split_descriptors;
   split_map->DeprecateTarget(
@@ -7343,6 +7348,36 @@ MaybeObject* DescriptorArray::Merge(int verbatim,
 }
 
 
+// Checks whether a merge of |this| and |other| would return a copy of |this|.
+bool DescriptorArray::IsMoreGeneralThan(int verbatim,
+                                        int valid,
+                                        int new_size,
+                                        DescriptorArray* other) {
+  ASSERT(verbatim <= valid);
+  ASSERT(valid <= new_size);
+  if (valid != new_size) return false;
+
+  for (int descriptor = verbatim; descriptor < valid; descriptor++) {
+    PropertyDetails details = GetDetails(descriptor);
+    PropertyDetails other_details = other->GetDetails(descriptor);
+    if (details.type() != other_details.type()) {
+      if (details.type() != FIELD ||
+          other_details.type() != CONSTANT_FUNCTION) {

[danno, 2013/05/02 14:26:41]

What about when other_details is a FIELD and details is a CONSTANT_FUNCTION?

[Toon Verwaest, 2013/05/02 15:15:57]

In that case |this| is not a valid updated version of |other|, and we'll have to
Merge again.

On 2013/05/02 14:26:41, danno wrote:

+        return false;
+      }
+    } else if (details.type() == CONSTANT_FUNCTION) {
+      if (GetValue(descriptor) != other->GetValue(descriptor)) {
+        return false;
+      }
+    } else if (!other_details.representation().fits(details.representation())) {

[danno, 2013/05/02 14:26:41]

So, merge doesn't seem to be commutative? Shouldn't your comments be more of the
phrasing "merge of 'this' _into_ 'other'"?

+      return false;
+    }
+  }
+
+  return true;
+}
+
+
 // We need the whiteness witness since sort will reshuffle the entries in the
 // descriptor array. If the descriptor array were to be black, the shuffling
 // would move a slot that was already recorded as pointing into an evacuation