Adding fast path for generalizing maps.

src/objects.cc

 // Copyright 2013 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 2442 matching lines @@
     return CopyGeneralizeAllRepresentations();
   }
 
   int verbatim = root_map->NumberOfOwnDescriptors();
 
   Map* updated = root_map->FindUpdatedMap(
       verbatim, descriptors, old_descriptors);
   // 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 =
       new_descriptors->GetDetails(modify_index).representation();
   new_representation = new_representation.generalize(old_reprepresentation);
   new_descriptors->SetRepresentation(modify_index, new_representation);
 
   Map* split_map = root_map->FindLastMatchMap(
       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(
       old_descriptors->GetKey(descriptor), new_descriptors);
 
   Map* new_map = split_map;
   // Add missing transitions.
   for (; descriptor < descriptors; descriptor++) {
     MaybeObject* maybe_map = new_map->CopyInstallDescriptors(
         descriptor, new_descriptors);
@@ skipping 4843 matching lines @@
     } else {
       result->CopyFrom(descriptor, other, descriptor, witness);
     }
   }
 
   result->Sort();
   return result;
 }
 
 
+// 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
 // candidate. This would result in missing updates upon evacuation.
 void DescriptorArray::Sort() {
   // In-place heap sort.
   int len = number_of_descriptors();
   // Reset sorting since the descriptor array might contain invalid pointers.
   for (int i = 0; i < len; ++i) SetSortedKey(i, i);
   // Bottom-up max-heap construction.
@@ skipping 7734 matching lines @@
   set_year(Smi::FromInt(year), SKIP_WRITE_BARRIER);
   set_month(Smi::FromInt(month), SKIP_WRITE_BARRIER);
   set_day(Smi::FromInt(day), SKIP_WRITE_BARRIER);
   set_weekday(Smi::FromInt(weekday), SKIP_WRITE_BARRIER);
   set_hour(Smi::FromInt(hour), SKIP_WRITE_BARRIER);
   set_min(Smi::FromInt(min), SKIP_WRITE_BARRIER);
   set_sec(Smi::FromInt(sec), SKIP_WRITE_BARRIER);
 }
 
 } }  // namespace v8::internal