Reapply descriptor array sharing.

This reverts commit 12669

Review URL: https://chromiumcodereview.appspot.com/11093026

git-svn-id: https://v8.googlecode.com/svn/trunk@12683 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/arm/full-codegen-arm.cc

@@ -2729,26 +2729,31 @@ void FullCodeGenerator::EmitIsStringWrapperSafeForDefaultValueOf(
   __ b(eq, if_false);
 
   // Look for valueOf symbol in the descriptor array, and indicate false if
-  // found. The type is not checked, so if it is a transition it is a false
-  // negative.
-  __ LoadInstanceDescriptors(r1, r4, r3);
-  __ ldr(r3, FieldMemOperand(r4, FixedArray::kLengthOffset));
-  // r4: descriptor array
-  // r3: length of descriptor array
-  // Calculate the end of the descriptor array.
+  // found. Since we omit an enumeration index check, if it is added via a
+  // transition that shares its descriptor array, this is a false positive.
+  Label entry, loop, done;
+
+  // Skip loop if no descriptors are valid.
+  __ NumberOfOwnDescriptors(r3, r1);
+  __ cmp(r3, Operand(0));
+  __ b(eq, &done);
+
+  __ LoadInstanceDescriptors(r1, r4, r2);
+  // r4: descriptor array.
+  // r3: valid entries in the descriptor array.
   STATIC_ASSERT(kSmiTag == 0);
   STATIC_ASSERT(kSmiTagSize == 1);
   STATIC_ASSERT(kPointerSize == 4);
-  __ add(r2, r4, Operand(FixedArray::kHeaderSize - kHeapObjectTag));
+  __ mov(ip, Operand(DescriptorArray::kDescriptorSize));
+  __ mul(r3, r3, ip);
+  // Calculate location of the first key name.
+  __ add(r4, r4, Operand(DescriptorArray::kFirstOffset - kHeapObjectTag));
+  // Calculate the end of the descriptor array.
+  __ mov(r2, r4);
   __ add(r2, r2, Operand(r3, LSL, kPointerSizeLog2 - kSmiTagSize));
 
-  // Calculate location of the first key name.
-  __ add(r4,
-         r4,
-         Operand(DescriptorArray::kFirstOffset - kHeapObjectTag));
   // Loop through all the keys in the descriptor array. If one of these is the
   // symbol valueOf the result is false.
-  Label entry, loop;
   // The use of ip to store the valueOf symbol asumes that it is not otherwise
   // used in the loop below.
   __ mov(ip, Operand(FACTORY->value_of_symbol()));
@@ -2762,7 +2767,8 @@ void FullCodeGenerator::EmitIsStringWrapperSafeForDefaultValueOf(
   __ cmp(r4, Operand(r2));
   __ b(ne, &loop);
 
-  // If a valueOf property is not found on the object check that it's
+  __ bind(&done);
+  // If a valueOf property is not found on the object check that its
   // prototype is the un-modified String prototype. If not result is false.
   __ ldr(r2, FieldMemOperand(r1, Map::kPrototypeOffset));
   __ JumpIfSmi(r2, if_false);

src/arm/macro-assembler-arm.cc

@@ -3703,20 +3703,37 @@ void MacroAssembler::LoadInstanceDescriptors(Register map,
   Register temp = descriptors;
   ldr(temp, FieldMemOperand(map, Map::kTransitionsOrBackPointerOffset));
 
-  Label ok, fail;
+  Label ok, fail, load_from_back_pointer;
   CheckMap(temp,
            scratch,
            isolate()->factory()->fixed_array_map(),
            &fail,
            DONT_DO_SMI_CHECK);
-  ldr(descriptors, FieldMemOperand(temp, TransitionArray::kDescriptorsOffset));
+  ldr(temp, FieldMemOperand(temp, TransitionArray::kDescriptorsPointerOffset));
+  ldr(descriptors, FieldMemOperand(temp, JSGlobalPropertyCell::kValueOffset));
   jmp(&ok);
+
   bind(&fail);
+  CompareRoot(temp, Heap::kUndefinedValueRootIndex);
+  b(ne, &load_from_back_pointer);
   mov(descriptors, Operand(FACTORY->empty_descriptor_array()));
+  jmp(&ok);
+
+  bind(&load_from_back_pointer);
+  ldr(temp, FieldMemOperand(temp, Map::kTransitionsOrBackPointerOffset));
+  ldr(temp, FieldMemOperand(temp, TransitionArray::kDescriptorsPointerOffset));
+  ldr(descriptors, FieldMemOperand(temp, JSGlobalPropertyCell::kValueOffset));
+
   bind(&ok);
 }
 
 
+void MacroAssembler::NumberOfOwnDescriptors(Register dst, Register map) {
+  ldr(dst, FieldMemOperand(map, Map::kBitFieldOffset));
+  DecodeField<Map::NumberOfOwnDescriptorsBits>(dst);
+}
+
+
 void MacroAssembler::EnumLength(Register dst, Register map) {
   STATIC_ASSERT(Map::EnumLengthBits::kShift == 0);
   ldr(dst, FieldMemOperand(map, Map::kBitField3Offset));

src/arm/macro-assembler-arm.h

@@ -1273,6 +1273,15 @@ class MacroAssembler: public Assembler {
                                Register descriptors,
                                Register scratch);
   void EnumLength(Register dst, Register map);
+  void NumberOfOwnDescriptors(Register dst, Register map);
+
+  template<typename Field>
+  void DecodeField(Register reg) {
+    static const int shift = Field::kShift;
+    static const int mask = (Field::kMask >> shift) << kSmiTagSize;
+    mov(reg, Operand(reg, LSR, shift));
+    and_(reg, reg, Operand(mask));
+  }
 
   // Activation support.
   void EnterFrame(StackFrame::Type type);

src/bootstrapper.cc

@@ -637,7 +637,7 @@ static void SetAccessors(Handle<Map> map,
                          Handle<String> name,
                          Handle<JSFunction> func) {
   DescriptorArray* descs = map->instance_descriptors();
-  int number = descs->Search(*name);
+  int number = descs->SearchWithCache(*name, *map);
   AccessorPair* accessors = AccessorPair::cast(descs->GetValue(number));
   accessors->set_getter(*func);
   accessors->set_setter(*func);
@@ -1774,7 +1774,8 @@ bool Genesis::InstallNatives() {
       Handle<DescriptorArray> array_descriptors(
           array_function->initial_map()->instance_descriptors());
       String* length = heap()->length_symbol();
-      int old = array_descriptors->SearchWithCache(length);
+      int old = array_descriptors->SearchWithCache(
+          length, array_function->initial_map());
       ASSERT(old != DescriptorArray::kNotFound);
       CallbacksDescriptor desc(length,
                                array_descriptors->GetValue(old),

src/handles.cc

@@ -705,24 +705,46 @@ Handle<JSArray> GetKeysFor(Handle<JSReceiver> object, bool* threw) {
 }
 
 
+Handle<FixedArray> ReduceFixedArrayTo(Handle<FixedArray> array, int length) {
+  ASSERT(array->length() >= length);
+  if (array->length() == length) return array;
+
+  Handle<FixedArray> new_array =
+      array->GetIsolate()->factory()->NewFixedArray(length);
+  for (int i = 0; i < length; ++i) new_array->set(i, array->get(i));
+  return new_array;
+}
+
+
 Handle<FixedArray> GetEnumPropertyKeys(Handle<JSObject> object,
                                        bool cache_result) {
   Isolate* isolate = object->GetIsolate();
   if (object->HasFastProperties()) {
     if (object->map()->instance_descriptors()->HasEnumCache()) {
       int own_property_count = object->map()->EnumLength();
-
-      // Mark that we have an enum cache if we are allowed to cache it.
-      if (cache_result && own_property_count == Map::kInvalidEnumCache) {
-        int num_enum = object->map()->NumberOfDescribedProperties(DONT_ENUM);
-        object->map()->SetEnumLength(num_enum);
+      // If we have an enum cache, but the enum length of the given map is set
+      // to kInvalidEnumCache, this means that the map itself has never used the
+      // present enum cache. The first step to using the cache is to set the
+      // enum length of the map by counting the number of own descriptors that
+      // are not DONT_ENUM.
+      if (own_property_count == Map::kInvalidEnumCache) {
+        own_property_count = object->map()->NumberOfDescribedProperties(
+            OWN_DESCRIPTORS, DONT_ENUM);
+
+        if (cache_result) object->map()->SetEnumLength(own_property_count);
       }
 
       DescriptorArray* desc = object->map()->instance_descriptors();
       Handle<FixedArray> keys(FixedArray::cast(desc->GetEnumCache()), isolate);
 
-      isolate->counters()->enum_cache_hits()->Increment();
-      return keys;
+      // In case the number of properties required in the enum are actually
+      // present, we can reuse the enum cache. Otherwise, this means that the
+      // enum cache was generated for a previous (smaller) version of the
+      // Descriptor Array. In that case we regenerate the enum cache.
+      if (own_property_count <= keys->length()) {
+        isolate->counters()->enum_cache_hits()->Increment();
+        return ReduceFixedArrayTo(keys, own_property_count);
+      }
     }
 
     Handle<Map> map(object->map());
@@ -734,19 +756,22 @@ Handle<FixedArray> GetEnumPropertyKeys(Handle<JSObject> object,
     }
 
     isolate->counters()->enum_cache_misses()->Increment();
-
-    int num_enum = map->NumberOfDescribedProperties(DONT_ENUM);
+    int num_enum = map->NumberOfDescribedProperties(ALL_DESCRIPTORS, DONT_ENUM);
 
     Handle<FixedArray> storage = isolate->factory()->NewFixedArray(num_enum);
     Handle<FixedArray> indices = isolate->factory()->NewFixedArray(num_enum);
 
     Handle<DescriptorArray> descs =
         Handle<DescriptorArray>(object->map()->instance_descriptors(), isolate);
 
+    int real_size = map->NumberOfOwnDescriptors();
+    int enum_size = 0;
     int index = 0;
+
     for (int i = 0; i < descs->number_of_descriptors(); i++) {
       PropertyDetails details = descs->GetDetails(i);
       if (!details.IsDontEnum()) {
+        if (i < real_size) ++enum_size;
         storage->set(index, descs->GetKey(i));
         if (!indices.is_null()) {
           if (details.type() != FIELD) {
@@ -773,9 +798,10 @@ Handle<FixedArray> GetEnumPropertyKeys(Handle<JSObject> object,
                        indices.is_null() ? Object::cast(Smi::FromInt(0))
                                          : Object::cast(*indices));
     if (cache_result) {
-      object->map()->SetEnumLength(index);
+      object->map()->SetEnumLength(enum_size);
     }
-    return storage;
+
+    return ReduceFixedArrayTo(storage, enum_size);
   } else {
     Handle<StringDictionary> dictionary(object->property_dictionary());
 

src/handles.h

@@ -276,6 +276,7 @@ Handle<FixedArray> GetKeysInFixedArrayFor(Handle<JSReceiver> object,
                                           KeyCollectionType type,
                                           bool* threw);
 Handle<JSArray> GetKeysFor(Handle<JSReceiver> object, bool* threw);
+Handle<FixedArray> ReduceFixedArrayTo(Handle<FixedArray> array, int length);
 Handle<FixedArray> GetEnumPropertyKeys(Handle<JSObject> object,
                                        bool cache_result);
 

src/heap.cc

@@ -2052,7 +2052,9 @@ MaybeObject* Heap::AllocatePartialMap(InstanceType instance_type,
   reinterpret_cast<Map*>(result)->set_unused_property_fields(0);
   reinterpret_cast<Map*>(result)->set_bit_field(0);
   reinterpret_cast<Map*>(result)->set_bit_field2(0);
-  reinterpret_cast<Map*>(result)->set_bit_field3(0);
+  int bit_field3 = Map::EnumLengthBits::encode(Map::kInvalidEnumCache) |
+                   Map::OwnsDescriptors::encode(true);
+  reinterpret_cast<Map*>(result)->set_bit_field3(bit_field3);
   return result;
 }
 
@@ -2079,7 +2081,8 @@ MaybeObject* Heap::AllocateMap(InstanceType instance_type,
   map->set_unused_property_fields(0);
   map->set_bit_field(0);
   map->set_bit_field2(1 << Map::kIsExtensible);
-  int bit_field3 = Map::EnumLengthBits::encode(Map::kInvalidEnumCache);
+  int bit_field3 = Map::EnumLengthBits::encode(Map::kInvalidEnumCache) |
+                   Map::OwnsDescriptors::encode(true);
   map->set_bit_field3(bit_field3);
   map->set_elements_kind(elements_kind);
 
@@ -7123,7 +7126,7 @@ void KeyedLookupCache::Clear() {
 
 
 void DescriptorLookupCache::Clear() {
-  for (int index = 0; index < kLength; index++) keys_[index].array = NULL;
+  for (int index = 0; index < kLength; index++) keys_[index].source = NULL;
 }
 
 

src/heap.h

@@ -2376,29 +2376,29 @@ class KeyedLookupCache {
 };
 
 
-// Cache for mapping (array, property name) into descriptor index.
+// Cache for mapping (map, property name) into descriptor index.
 // The cache contains both positive and negative results.
 // Descriptor index equals kNotFound means the property is absent.
 // Cleared at startup and prior to any gc.
 class DescriptorLookupCache {
  public:
   // Lookup descriptor index for (map, name).
   // If absent, kAbsent is returned.
-  int Lookup(DescriptorArray* array, String* name) {
+  int Lookup(Map* source, String* name) {
     if (!StringShape(name).IsSymbol()) return kAbsent;
-    int index = Hash(array, name);
+    int index = Hash(source, name);
     Key& key = keys_[index];
-    if ((key.array == array) && (key.name == name)) return results_[index];
+    if ((key.source == source) && (key.name == name)) return results_[index];
     return kAbsent;
   }
 
   // Update an element in the cache.
-  void Update(DescriptorArray* array, String* name, int result) {
+  void Update(Map* source, String* name, int result) {
     ASSERT(result != kAbsent);
     if (StringShape(name).IsSymbol()) {
-      int index = Hash(array, name);
+      int index = Hash(source, name);
       Key& key = keys_[index];
-      key.array = array;
+      key.source = source;
       key.name = name;
       results_[index] = result;
     }
@@ -2412,26 +2412,26 @@ class DescriptorLookupCache {
  private:
   DescriptorLookupCache() {
     for (int i = 0; i < kLength; ++i) {
-      keys_[i].array = NULL;
+      keys_[i].source = NULL;
       keys_[i].name = NULL;
       results_[i] = kAbsent;
     }
   }
 
-  static int Hash(DescriptorArray* array, String* name) {
+  static int Hash(Object* source, String* name) {
     // Uses only lower 32 bits if pointers are larger.
-    uint32_t array_hash =
-        static_cast<uint32_t>(reinterpret_cast<uintptr_t>(array))
+    uint32_t source_hash =
+        static_cast<uint32_t>(reinterpret_cast<uintptr_t>(source))
             >> kPointerSizeLog2;
     uint32_t name_hash =
         static_cast<uint32_t>(reinterpret_cast<uintptr_t>(name))
             >> kPointerSizeLog2;
-    return (array_hash ^ name_hash) % kLength;
+    return (source_hash ^ name_hash) % kLength;
   }
 
   static const int kLength = 64;
   struct Key {
-    DescriptorArray* array;
+    Map* source;
     String* name;
   };
 

src/ia32/full-codegen-ia32.cc

@@ -2669,22 +2669,28 @@ void FullCodeGenerator::EmitIsStringWrapperSafeForDefaultValueOf(
   __ j(equal, if_false);
 
   // Look for valueOf symbol in the descriptor array, and indicate false if
-  // found. The type is not checked, so if it is a transition it is a false
-  // negative.
+  // found. Since we omit an enumeration index check, if it is added via a
+  // transition that shares its descriptor array, this is a false positive.
+  Label entry, loop, done;
+
+  // Skip loop if no descriptors are valid.
+  __ NumberOfOwnDescriptors(ecx, ebx);
+  __ cmp(ecx, 0);
+  __ j(equal, &done);
+
   __ LoadInstanceDescriptors(ebx, ebx);
-  __ mov(ecx, FieldOperand(ebx, FixedArray::kLengthOffset));
-  // ebx: descriptor array
-  // ecx: length of descriptor array
+  // ebx: descriptor array.
+  // ecx: valid entries in the descriptor array.
   // Calculate the end of the descriptor array.
   STATIC_ASSERT(kSmiTag == 0);
   STATIC_ASSERT(kSmiTagSize == 1);
   STATIC_ASSERT(kPointerSize == 4);
-  __ lea(ecx, Operand(ebx, ecx, times_2, FixedArray::kHeaderSize));
+  __ imul(ecx, ecx, DescriptorArray::kDescriptorSize);
+  __ lea(ecx, Operand(ebx, ecx, times_2, DescriptorArray::kFirstOffset));
   // Calculate location of the first key name.
   __ add(ebx, Immediate(DescriptorArray::kFirstOffset));
   // Loop through all the keys in the descriptor array. If one of these is the
   // symbol valueOf the result is false.
-  Label entry, loop;
   __ jmp(&entry);
   __ bind(&loop);
   __ mov(edx, FieldOperand(ebx, 0));
@@ -2695,10 +2701,12 @@ void FullCodeGenerator::EmitIsStringWrapperSafeForDefaultValueOf(
   __ cmp(ebx, ecx);
   __ j(not_equal, &loop);
 
+  __ bind(&done);
+
   // Reload map as register ebx was used as temporary above.
   __ mov(ebx, FieldOperand(eax, HeapObject::kMapOffset));
 
-  // If a valueOf property is not found on the object check that it's
+  // If a valueOf property is not found on the object check that its
   // prototype is the un-modified String prototype. If not result is false.
   __ mov(ecx, FieldOperand(ebx, Map::kPrototypeOffset));
   __ JumpIfSmi(ecx, if_false);