Separate DescriptorArray from WeakFixedArray

This patch gives DescriptorArray its own visitor id and its own layout that is independent from the layout of WeakFixedArray. This allows us to use raw 16-bit integers for keeping track of the number of descriptors (total, non-slack, and marked). As a side-effect, we save one word per descriptor array on 64-bit. v8:8486 Change-Id: If8389dde446319e5b3491abc948b52539dba235c Reviewed-on: https://chromium-review.googlesource.com/c/1349245 Commit-Queue: Ulan Degenbaev <[email protected]> Reviewed-by: Igor Sheludko <[email protected]> Cr-Commit-Position: refs/heads/master@{#57845}
v8 · Nov 26, 2018 · 1ad0cd5 · 1ad0cd5
1 parent 0397f78
commit 1ad0cd5
Show file tree

Hide file tree

Showing 36 changed files with 590 additions and 448 deletions.
diff --git a/src/api.cc b/src/api.cc
@@ -4399,7 +4399,7 @@ MaybeLocal<Array> v8::Object::GetPropertyNames(
       accumulator.GetKeys(static_cast<i::GetKeysConversion>(key_conversion));
   DCHECK(self->map()->EnumLength() == i::kInvalidEnumCacheSentinel ||
          self->map()->EnumLength() == 0 ||
-         self->map()->instance_descriptors()->GetEnumCache()->keys() != *value);
+         self->map()->instance_descriptors()->enum_cache()->keys() != *value);
   auto result = isolate->factory()->NewJSArrayWithElements(value);
   RETURN_ESCAPED(Utils::ToLocal(result));
 }

diff --git a/src/base/atomicops.h b/src/base/atomicops.h
@@ -40,6 +40,7 @@ namespace v8 {
 namespace base {
 
 typedef char Atomic8;
+typedef int16_t Atomic16;
 typedef int32_t Atomic32;
 #if defined(V8_HOST_ARCH_64_BIT)
 // We need to be able to go between Atomic64 and AtomicWord implicitly.  This
@@ -97,10 +98,12 @@ Atomic32 Release_CompareAndSwap(volatile Atomic32* ptr,
 
 void SeqCst_MemoryFence();
 void Relaxed_Store(volatile Atomic8* ptr, Atomic8 value);
+void Relaxed_Store(volatile Atomic16* ptr, Atomic16 value);
 void Relaxed_Store(volatile Atomic32* ptr, Atomic32 value);
 void Release_Store(volatile Atomic32* ptr, Atomic32 value);
 
 Atomic8 Relaxed_Load(volatile const Atomic8* ptr);
+Atomic16 Relaxed_Load(volatile const Atomic16* ptr);
 Atomic32 Relaxed_Load(volatile const Atomic32* ptr);
 Atomic32 Acquire_Load(volatile const Atomic32* ptr);
 

diff --git a/src/base/atomicops_internals_portable.h b/src/base/atomicops_internals_portable.h
@@ -98,6 +98,10 @@ inline void Relaxed_Store(volatile Atomic8* ptr, Atomic8 value) {
   __atomic_store_n(ptr, value, __ATOMIC_RELAXED);
 }
 
+inline void Relaxed_Store(volatile Atomic16* ptr, Atomic16 value) {
+  __atomic_store_n(ptr, value, __ATOMIC_RELAXED);
+}
+
 inline void Relaxed_Store(volatile Atomic32* ptr, Atomic32 value) {
   __atomic_store_n(ptr, value, __ATOMIC_RELAXED);
 }
@@ -110,6 +114,10 @@ inline Atomic8 Relaxed_Load(volatile const Atomic8* ptr) {
   return __atomic_load_n(ptr, __ATOMIC_RELAXED);
 }
 
+inline Atomic16 Relaxed_Load(volatile const Atomic16* ptr) {
+  return __atomic_load_n(ptr, __ATOMIC_RELAXED);
+}
+
 inline Atomic32 Relaxed_Load(volatile const Atomic32* ptr) {
   return __atomic_load_n(ptr, __ATOMIC_RELAXED);
 }

diff --git a/src/base/atomicops_internals_std.h b/src/base/atomicops_internals_std.h
@@ -86,6 +86,11 @@ inline void Relaxed_Store(volatile Atomic8* ptr, Atomic8 value) {
                              std::memory_order_relaxed);
 }
 
+inline void Relaxed_Store(volatile Atomic16* ptr, Atomic16 value) {
+  std::atomic_store_explicit(helper::to_std_atomic(ptr), value,
+                             std::memory_order_relaxed);
+}
+
 inline void Relaxed_Store(volatile Atomic32* ptr, Atomic32 value) {
   std::atomic_store_explicit(helper::to_std_atomic(ptr), value,
                              std::memory_order_relaxed);
@@ -101,6 +106,11 @@ inline Atomic8 Relaxed_Load(volatile const Atomic8* ptr) {
                                    std::memory_order_relaxed);
 }
 
+inline Atomic16 Relaxed_Load(volatile const Atomic16* ptr) {
+  return std::atomic_load_explicit(helper::to_std_atomic_const(ptr),
+                                   std::memory_order_relaxed);
+}
+
 inline Atomic32 Relaxed_Load(volatile const Atomic32* ptr) {
   return std::atomic_load_explicit(helper::to_std_atomic_const(ptr),
                                    std::memory_order_relaxed);

diff --git a/src/builtins/builtins-function-gen.cc b/src/builtins/builtins-function-gen.cc
@@ -47,12 +47,12 @@ TF_BUILTIN(FastFunctionPrototypeBind, CodeStubAssembler) {
   Comment("Check descriptor array length");
   TNode<DescriptorArray> descriptors = LoadMapDescriptors(receiver_map);
   // Minimum descriptor array length required for fast path.
-  const int min_descriptors_length = DescriptorArray::LengthFor(i::Max(
-      JSFunction::kLengthDescriptorIndex, JSFunction::kNameDescriptorIndex));
-  TNode<Smi> descriptors_length = LoadWeakFixedArrayLength(descriptors);
-  GotoIf(SmiLessThanOrEqual(descriptors_length,
-                            SmiConstant(min_descriptors_length)),
-         &slow);
+  const int min_nof_descriptors = i::Max(JSFunction::kLengthDescriptorIndex,
+                                         JSFunction::kNameDescriptorIndex);
+  TNode<Int32T> nof_descriptors = LoadNumberOfDescriptors(descriptors);
+  GotoIf(
+      Int32LessThanOrEqual(nof_descriptors, Int32Constant(min_nof_descriptors)),
+      &slow);
 
   // Check whether the length and name properties are still present as
   // AccessorInfo objects. In that case, their value can be recomputed even if

diff --git a/src/code-stub-assembler.cc b/src/code-stub-assembler.cc
@@ -1605,6 +1605,13 @@ TNode<IntPtrT> CodeStubAssembler::LoadAndUntagWeakFixedArrayLength(
   return LoadAndUntagObjectField(array, WeakFixedArray::kLengthOffset);
 }
 
+TNode<Int32T> CodeStubAssembler::LoadNumberOfDescriptors(
+    TNode<DescriptorArray> array) {
+  return UncheckedCast<Int32T>(
+      LoadObjectField(array, DescriptorArray::kNumberOfDescriptorsOffset,
+                      MachineType::Int16()));
+}
+
 TNode<Int32T> CodeStubAssembler::LoadMapBitField(SloppyTNode<Map> map) {
   CSA_SLOW_ASSERT(this, IsMap(map));
   return UncheckedCast<Int32T>(
@@ -1961,7 +1968,8 @@ TNode<IntPtrT> CodeStubAssembler::LoadArrayLength(TNode<PropertyArray> array) {
 template <>
 TNode<IntPtrT> CodeStubAssembler::LoadArrayLength(
     TNode<DescriptorArray> array) {
-  return LoadAndUntagWeakFixedArrayLength(array);
+  return IntPtrMul(ChangeInt32ToIntPtr(LoadNumberOfDescriptors(array)),
+                   IntPtrConstant(DescriptorArray::kEntrySize));
 }
 
 template <>
@@ -8701,7 +8709,8 @@ void CodeStubAssembler::LookupLinear(TNode<Name> unique_name,
                                      TVariable<IntPtrT>* var_name_index,
                                      Label* if_not_found) {
   static_assert(std::is_base_of<FixedArray, Array>::value ||
-                    std::is_base_of<WeakFixedArray, Array>::value,
+                    std::is_base_of<WeakFixedArray, Array>::value ||
+                    std::is_base_of<DescriptorArray, Array>::value,
                 "T must be a descendant of FixedArray or a WeakFixedArray");
   Comment("LookupLinear");
   TNode<IntPtrT> first_inclusive = IntPtrConstant(Array::ToKeyIndex(0));
@@ -8725,9 +8734,7 @@ void CodeStubAssembler::LookupLinear(TNode<Name> unique_name,
 template <>
 TNode<Uint32T> CodeStubAssembler::NumberOfEntries<DescriptorArray>(
     TNode<DescriptorArray> descriptors) {
-  return Unsigned(LoadAndUntagToWord32ArrayElement(
-      descriptors, WeakFixedArray::kHeaderSize,
-      IntPtrConstant(DescriptorArray::kDescriptorLengthIndex)));
+  return Unsigned(LoadNumberOfDescriptors(descriptors));
 }
 
 template <>
@@ -8780,9 +8787,9 @@ TNode<Uint32T> CodeStubAssembler::GetSortedKeyIndex<TransitionArray>(
 template <typename Array>
 TNode<Name> CodeStubAssembler::GetKey(TNode<Array> array,
                                       TNode<Uint32T> entry_index) {
-  static_assert(std::is_base_of<FixedArray, Array>::value ||
-                    std::is_base_of<WeakFixedArray, Array>::value,
-                "T must be a descendant of FixedArray or a TransitionArray");
+  static_assert(std::is_base_of<TransitionArray, Array>::value ||
+                    std::is_base_of<DescriptorArray, Array>::value,
+                "T must be a descendant of DescriptorArray or TransitionArray");
   const int key_offset = Array::ToKeyIndex(0) * kPointerSize;
   TNode<MaybeObject> element =
       LoadArrayElement(array, Array::kHeaderSize,
@@ -13703,8 +13710,8 @@ void CodeStubAssembler::GotoIfInitialPrototypePropertiesModified(
     for (int i = 0; i < properties.length(); i++) {
       // Assert the descriptor index is in-bounds.
       int descriptor = properties[i].descriptor_index;
-      CSA_ASSERT(this, SmiLessThan(SmiConstant(descriptor),
-                                   LoadWeakFixedArrayLength(descriptors)));
+      CSA_ASSERT(this, Int32LessThan(Int32Constant(descriptor),
+                                     LoadNumberOfDescriptors(descriptors)));
       // Assert that the name is correct. This essentially checks that
       // the descriptor index corresponds to the insertion order in
       // the bootstrapper.

diff --git a/src/code-stub-assembler.h b/src/code-stub-assembler.h
@@ -931,6 +931,8 @@ class V8_EXPORT_PRIVATE CodeStubAssembler
   TNode<Smi> LoadWeakFixedArrayLength(TNode<WeakFixedArray> array);
   TNode<IntPtrT> LoadAndUntagWeakFixedArrayLength(
       SloppyTNode<WeakFixedArray> array);
+  // Load the number of descriptors in DescriptorArray.
+  TNode<Int32T> LoadNumberOfDescriptors(TNode<DescriptorArray> array);
   // Load the bit field of a Map.
   TNode<Int32T> LoadMapBitField(SloppyTNode<Map> map);
   // Load bit field 2 of a map.

diff --git a/src/heap/factory.cc b/src/heap/factory.cc
@@ -343,10 +343,6 @@ Handle<T> Factory::NewWeakFixedArrayWithMap(RootIndex map_root_index,
 template Handle<FixedArray> Factory::NewFixedArrayWithMap<FixedArray>(
     RootIndex, int, PretenureFlag);
 
-template Handle<DescriptorArray>
-Factory::NewWeakFixedArrayWithMap<DescriptorArray>(RootIndex, int,
-                                                   PretenureFlag);
-
 Handle<FixedArray> Factory::NewFixedArray(int length, PretenureFlag pretenure) {
   DCHECK_LE(0, length);
   if (length == 0) return empty_fixed_array();
@@ -1857,6 +1853,22 @@ Handle<PropertyCell> Factory::NewPropertyCell(Handle<Name> name,
   return cell;
 }
 
+Handle<DescriptorArray> Factory::NewDescriptorArray(int number_of_descriptors,
+                                                    int slack) {
+  int number_of_all_descriptors = number_of_descriptors + slack;
+  // Zero-length case must be handled outside.
+  DCHECK_LT(0, number_of_all_descriptors);
+  int size = DescriptorArray::SizeFor(number_of_all_descriptors);
+  DCHECK_LT(size, kMaxRegularHeapObjectSize);
+  HeapObject* obj =
+      isolate()->heap()->AllocateRawWithRetryOrFail(size, OLD_SPACE);
+  obj->set_map_after_allocation(*descriptor_array_map(), SKIP_WRITE_BARRIER);
+  DescriptorArray* array = DescriptorArray::cast(obj);
+  array->Initialize(*empty_enum_cache(), *undefined_value(),
+                    number_of_descriptors, slack);
+  return Handle<DescriptorArray>(array, isolate());
+}
+
 Handle<TransitionArray> Factory::NewTransitionArray(int number_of_transitions,
                                                     int slack) {
   int capacity = TransitionArray::LengthFor(number_of_transitions + slack);

diff --git a/src/heap/factory.h b/src/heap/factory.h
@@ -483,6 +483,8 @@ class V8_EXPORT_PRIVATE Factory {
   Handle<FeedbackCell> NewManyClosuresCell(Handle<HeapObject> value);
   Handle<FeedbackCell> NewNoFeedbackCell();
 
+  Handle<DescriptorArray> NewDescriptorArray(int number_of_entries,
+                                             int slack = 0);
   Handle<TransitionArray> NewTransitionArray(int number_of_transitions,
                                              int slack = 0);
 

diff --git a/src/heap/mark-compact-inl.h b/src/heap/mark-compact-inl.h
@@ -367,15 +367,16 @@ void MarkingVisitor<fixed_array_mode, retaining_path_mode,
   // just mark the entire descriptor array.
   if (!map->is_prototype_map()) {
     DescriptorArray* descriptors = map->instance_descriptors();
-    if (MarkObjectWithoutPush(map, descriptors) && descriptors->length() > 0) {
-      VisitPointers(descriptors, descriptors->data_start(),
-                    descriptors->GetDescriptorEndSlot(0));
+    if (MarkObjectWithoutPush(map, descriptors)) {
+      VisitPointers(descriptors, descriptors->GetFirstPointerSlot(),
+                    descriptors->GetDescriptorSlot(0));
     }
     int start = 0;
     int end = map->NumberOfOwnDescriptors();
     if (start < end) {
-      VisitPointers(descriptors, descriptors->GetDescriptorStartSlot(start),
-                    descriptors->GetDescriptorEndSlot(end));
+      VisitPointers(descriptors,
+                    MaybeObjectSlot(descriptors->GetDescriptorSlot(start)),
+                    MaybeObjectSlot(descriptors->GetDescriptorSlot(end)));
     }
   }
 

diff --git a/src/heap/mark-compact.cc b/src/heap/mark-compact.cc
@@ -2007,20 +2007,38 @@ bool MarkCompactCollector::CompactTransitionArray(
   return descriptors_owner_died;
 }
 
+void MarkCompactCollector::RightTrimDescriptorArray(DescriptorArray* array,
+                                                    int descriptors_to_trim) {
+  int old_nof_all_descriptors = array->number_of_all_descriptors();
+  int new_nof_all_descriptors = old_nof_all_descriptors - descriptors_to_trim;
+  DCHECK_LT(0, descriptors_to_trim);
+  DCHECK_LE(0, new_nof_all_descriptors);
+  Address start = array->GetDescriptorSlot(new_nof_all_descriptors).address();
+  Address end = array->GetDescriptorSlot(old_nof_all_descriptors).address();
+  RememberedSet<OLD_TO_NEW>::RemoveRange(MemoryChunk::FromHeapObject(array),
+                                         start, end,
+                                         SlotSet::PREFREE_EMPTY_BUCKETS);
+  RememberedSet<OLD_TO_OLD>::RemoveRange(MemoryChunk::FromHeapObject(array),
+                                         start, end,
+                                         SlotSet::PREFREE_EMPTY_BUCKETS);
+  heap()->CreateFillerObjectAt(start, static_cast<int>(end - start),
+                               ClearRecordedSlots::kNo);
+  array->set_number_of_all_descriptors(new_nof_all_descriptors);
+}
+
 void MarkCompactCollector::TrimDescriptorArray(Map map,
                                                DescriptorArray* descriptors) {
   int number_of_own_descriptors = map->NumberOfOwnDescriptors();
   if (number_of_own_descriptors == 0) {
     DCHECK(descriptors == ReadOnlyRoots(heap_).empty_descriptor_array());
     return;
   }
-
-  int number_of_descriptors = descriptors->number_of_descriptors_storage();
-  int to_trim = number_of_descriptors - number_of_own_descriptors;
+  // TODO(ulan): Trim only if slack is greater than some percentage threshold.
+  int to_trim =
+      descriptors->number_of_all_descriptors() - number_of_own_descriptors;
   if (to_trim > 0) {
-    heap_->RightTrimWeakFixedArray(descriptors,
-                                   to_trim * DescriptorArray::kEntrySize);
-    descriptors->SetNumberOfDescriptors(number_of_own_descriptors);
+    descriptors->set_number_of_descriptors(number_of_own_descriptors);
+    RightTrimDescriptorArray(descriptors, to_trim);
 
     TrimEnumCache(map, descriptors);
     descriptors->Sort();
@@ -2043,7 +2061,7 @@ void MarkCompactCollector::TrimEnumCache(Map map,
     live_enum = map->NumberOfEnumerableProperties();
   }
   if (live_enum == 0) return descriptors->ClearEnumCache();
-  EnumCache* enum_cache = descriptors->GetEnumCache();
+  EnumCache* enum_cache = descriptors->enum_cache();
 
   FixedArray keys = enum_cache->keys();
   int to_trim = keys->length() - live_enum;

diff --git a/src/heap/mark-compact.h b/src/heap/mark-compact.h
@@ -854,6 +854,9 @@ class MarkCompactCollector final : public MarkCompactCollectorBase {
 
   int NumberOfParallelEphemeronVisitingTasks(size_t elements);
 
+  void RightTrimDescriptorArray(DescriptorArray* array,
+                                int descriptors_to_trim);
+
   base::Mutex mutex_;
   base::Semaphore page_parallel_job_semaphore_;
 

diff --git a/src/heap/object-stats.cc b/src/heap/object-stats.cc
@@ -773,7 +773,7 @@ void ObjectStatsCollectorImpl::RecordVirtualMapDetails(Map map) {
   if (map->owns_descriptors() &&
       array != ReadOnlyRoots(heap_).empty_descriptor_array()) {
     // DescriptorArray has its own instance type.
-    EnumCache* enum_cache = array->GetEnumCache();
+    EnumCache* enum_cache = array->enum_cache();
     RecordSimpleVirtualObjectStats(array, enum_cache->keys(),
                                    ObjectStats::ENUM_CACHE_TYPE);
     RecordSimpleVirtualObjectStats(array, enum_cache->indices(),

diff --git a/src/heap/objects-visiting.h b/src/heap/objects-visiting.h
@@ -42,6 +42,7 @@ class WasmInstanceObject;
   V(CodeDataContainer, CodeDataContainer*)                                     \
   V(ConsString, ConsString*)                                                   \
   V(DataHandler, DataHandler*)                                                 \
+  V(DescriptorArray, DescriptorArray*)                                         \
   V(EmbedderDataArray, EmbedderDataArray)                                      \
   V(EphemeronHashTable, EphemeronHashTable)                                    \
   V(FeedbackCell, FeedbackCell*)                                               \

diff --git a/src/heap/setup-heap-internal.cc b/src/heap/setup-heap-internal.cc
@@ -310,19 +310,14 @@ bool Heap::CreateInitialMaps() {
 
   // Allocate the empty descriptor array.
   {
-    STATIC_ASSERT(DescriptorArray::kFirstIndex != 0);
-    int length = DescriptorArray::kFirstIndex;
-    int size = WeakFixedArray::SizeFor(length);
+    int size = DescriptorArray::SizeFor(0);
     if (!AllocateRaw(size, RO_SPACE).To(&obj)) return false;
     obj->set_map_after_allocation(roots.descriptor_array_map(),
                                   SKIP_WRITE_BARRIER);
-    DescriptorArray::cast(obj)->set_length(length);
+    DescriptorArray* array = DescriptorArray::cast(obj);
+    array->Initialize(roots.empty_enum_cache(), roots.undefined_value(), 0, 0);
   }
   set_empty_descriptor_array(DescriptorArray::cast(obj));
-  DescriptorArray::cast(obj)->SetNumberOfDescriptors(0);
-  WeakFixedArray::cast(obj)->Set(
-      DescriptorArray::kEnumCacheIndex,
-      MaybeObject::FromObject(roots.empty_enum_cache()));
 
   // Fix the instance_descriptors for the existing maps.
   FinalizePartialMap(roots.meta_map());

diff --git a/src/keys.cc b/src/keys.cc
@@ -285,7 +285,7 @@ Handle<FixedArray> ReduceFixedArrayTo(Isolate* isolate,
 Handle<FixedArray> GetFastEnumPropertyKeys(Isolate* isolate,
                                            Handle<JSObject> object) {
   Handle<Map> map(object->map(), isolate);
-  Handle<FixedArray> keys(map->instance_descriptors()->GetEnumCache()->keys(),
+  Handle<FixedArray> keys(map->instance_descriptors()->enum_cache()->keys(),
                           isolate);
 
   // Check if the {map} has a valid enum length, which implies that it
@@ -351,7 +351,8 @@ Handle<FixedArray> GetFastEnumPropertyKeys(Isolate* isolate,
     DCHECK_EQ(index, indices->length());
   }
 
-  DescriptorArray::SetEnumCache(descriptors, isolate, keys, indices);
+  DescriptorArray::InitializeOrChangeEnumCache(descriptors, isolate, keys,
+                                               indices);
   if (map->OnlyHasSimpleProperties()) map->SetEnumLength(enum_length);
 
   return keys;

diff --git a/src/map-updater.cc b/src/map-updater.cc
@@ -442,11 +442,13 @@ Handle<DescriptorArray> MapUpdater::BuildDescriptorArray() {
   // descriptors, with minimally the exact same size as the old descriptor
   // array.
   int new_slack =
-      Max(old_nof_, old_descriptors_->number_of_descriptors()) - old_nof_;
+      std::max<int>(old_nof_, old_descriptors_->number_of_descriptors()) -
+      old_nof_;
   Handle<DescriptorArray> new_descriptors =
       DescriptorArray::Allocate(isolate_, old_nof_, new_slack);
-  DCHECK(new_descriptors->length() > target_descriptors->length() ||
-         new_descriptors->NumberOfSlackDescriptors() > 0 ||
+  DCHECK(new_descriptors->number_of_all_descriptors() >
+             target_descriptors->number_of_all_descriptors() ||
+         new_descriptors->number_of_slack_descriptors() > 0 ||
          new_descriptors->number_of_descriptors() ==
              old_descriptors_->number_of_descriptors());
   DCHECK(new_descriptors->number_of_descriptors() == old_nof_);