Add a byte field to Map that tracks used in-object property fields.

The motivation for the new field is to provide race-free way to
iterate used in-object properties of a JSObject in concurrent marker.

This CL keeps the new field in sync with the unused_property_fields
and subsequent CL will remove unused_property_fields.

Bug: chromium:774644
Change-Id: I0971f079094d58d3a57415834c43c09427dacc77
Reviewed-on: https://chromium-review.googlesource.com/726639
Commit-Queue: Ulan Degenbaev <ulan@chromium.org>
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Reviewed-by: Tobias Tebbi <tebbi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#48795}

src/bootstrapper.cc

@@ -797,8 +797,12 @@ Handle<Map> CreateNonConstructorMap(Handle<Map> source_map,
   // Ensure the resulting map has prototype slot (it is necessary for storing
   // inital map even when the prototype property is not required).
   if (!map->has_prototype_slot()) {
+    // Re-set the unused property fields after changing the instance size.
+    // TODO(ulan): Do not change instance size after map creation.
+    int unused_property_fields = map->UnusedPropertyFields();
     map->set_instance_size(map->instance_size() + kPointerSize);
     map->set_has_prototype_slot(true);
+    map->SetInObjectUnusedPropertyFields(unused_property_fields);
   }
   map->set_is_constructor(false);
   Map::SetPrototype(map, prototype);
@@ -3323,9 +3327,13 @@ void Genesis::InitializeGlobal(Handle<JSGlobalObject> global_object,
     // not have a prototype property.
     Handle<Map> proxy_function_map =
         Map::Copy(isolate->strict_function_without_prototype_map(), "Proxy");
+    // Re-set the unused property fields after changing the instance size.
+    // TODO(ulan): Do not change instance size after map creation.
+    int unused_property_fields = proxy_function_map->UnusedPropertyFields();
     proxy_function_map->set_instance_size(JSFunction::kSizeWithPrototype);
     proxy_function_map->set_has_prototype_slot(true);
     proxy_function_map->set_is_constructor(true);
+    proxy_function_map->SetInObjectUnusedPropertyFields(unused_property_fields);
 
     Handle<String> name = factory->Proxy_string();
     Handle<Code> code(BUILTIN_CODE(isolate, ProxyConstructor));

src/compiler/js-native-context-specialization.cc

@@ -1912,7 +1912,7 @@ JSNativeContextSpecialization::BuildPropertyStore(
       // with this transitioning store.
       Handle<Map> original_map(Map::cast(transition_map->GetBackPointer()),
                                isolate());
-      if (original_map->unused_property_fields() == 0) {
+      if (original_map->UnusedPropertyFields() == 0) {
         DCHECK(!field_index.is_inobject());
 
         // Reallocate the properties {storage}.
@@ -2393,7 +2393,7 @@ Node* JSNativeContextSpecialization::BuildExtendPropertiesBackingStore(
   // difficult for escape analysis to get rid of the backing stores used
   // for intermediate states of chains of property additions. That makes
   // it unclear what the best approach is here.
-  DCHECK_EQ(0, map->unused_property_fields());
+  DCHECK_EQ(0, map->UnusedPropertyFields());
   // Compute the length of the old {properties} and the new properties.
   int length = map->NextFreePropertyIndex() - map->GetInObjectProperties();
   int new_length = length + JSObject::kFieldsAdded;

src/factory.cc

@@ -1921,7 +1921,7 @@ Handle<JSGlobalObject> Factory::NewJSGlobalObject(
 
   // Make sure we don't have a ton of pre-allocated slots in the
   // global objects. They will be unused once we normalize the object.
-  DCHECK_EQ(map->unused_property_fields(), 0);
+  DCHECK_EQ(map->UnusedPropertyFields(), 0);
   DCHECK_EQ(map->GetInObjectProperties(), 0);
 
   // Initial size of the backing store to avoid resize of the storage during

src/heap/heap.cc

@@ -2327,9 +2327,8 @@ AllocationResult Heap::AllocatePartialMap(InstanceType instance_type,
   }
   // GetVisitorId requires a properly initialized LayoutDescriptor.
   map->set_visitor_id(Map::GetVisitorId(map));
-  map->clear_unused();
   map->set_inobject_properties_or_constructor_function_index(0);
-  map->set_unused_property_fields(0);
+  map->SetInObjectUnusedPropertyFields(0);
   map->set_bit_field(0);
   map->set_bit_field2(0);
   int bit_field3 = Map::EnumLengthBits::encode(kInvalidEnumCacheSentinel) |
@@ -2355,14 +2354,13 @@ AllocationResult Heap::AllocateMap(InstanceType instance_type,
   map->set_prototype(null_value(), SKIP_WRITE_BARRIER);
   map->set_constructor_or_backpointer(null_value(), SKIP_WRITE_BARRIER);
   map->set_instance_size(instance_size);
-  map->clear_unused();
   map->set_inobject_properties_or_constructor_function_index(
       inobject_properties);
   map->set_dependent_code(DependentCode::cast(empty_fixed_array()),
                           SKIP_WRITE_BARRIER);
   map->set_weak_cell_cache(Smi::kZero);
   map->set_raw_transitions(Smi::kZero);
-  map->set_unused_property_fields(inobject_properties);
+  map->SetInObjectUnusedPropertyFields(inobject_properties);
   map->set_instance_descriptors(empty_descriptor_array());
   if (FLAG_unbox_double_fields) {
     map->set_layout_descriptor(LayoutDescriptor::FastPointerLayout());

src/ic/handler-configuration.cc

@@ -259,7 +259,7 @@ Handle<Object> StoreHandler::StoreTransition(Isolate* isolate,
       } else {
         DCHECK_EQ(kField, details.location());
         bool extend_storage = Map::cast(transition_map->GetBackPointer())
-                                  ->unused_property_fields() == 0;
+                                  ->UnusedPropertyFields() == 0;
 
         FieldIndex index =
             FieldIndex::ForDescriptor(*transition_map, descriptor);

src/objects-debug.cc

@@ -326,16 +326,15 @@ void JSObject::JSObjectVerify() {
     int actual_unused_property_fields = map()->GetInObjectProperties() +
                                         property_array()->length() -
                                         map()->NextFreePropertyIndex();
-    if (map()->unused_property_fields() != actual_unused_property_fields) {
+    if (map()->UnusedPropertyFields() != actual_unused_property_fields) {
       // There are two reasons why this can happen:
       // - in the middle of StoreTransitionStub when the new extended backing
       //   store is already set into the object and the allocation of the
       //   MutableHeapNumber triggers GC while the map isn't updated yet.
       // - deletion of the last property can leave additional backing store
       //   capacity behind.
-      CHECK_GT(actual_unused_property_fields, map()->unused_property_fields());
-      int delta =
-          actual_unused_property_fields - map()->unused_property_fields();
+      CHECK_GT(actual_unused_property_fields, map()->UnusedPropertyFields());
+      int delta = actual_unused_property_fields - map()->UnusedPropertyFields();
       CHECK_EQ(0, delta % JSObject::kFieldsAdded);
     }
     DescriptorArray* descriptors = map()->instance_descriptors();
@@ -432,7 +431,7 @@ void Map::DictionaryMapVerify() {
   CHECK(is_dictionary_map());
   CHECK_EQ(kInvalidEnumCacheSentinel, EnumLength());
   CHECK_EQ(GetHeap()->empty_descriptor_array(), instance_descriptors());
-  CHECK_EQ(0, unused_property_fields());
+  CHECK_EQ(0, UnusedPropertyFields());
   CHECK_EQ(Map::GetVisitorId(this), visitor_id());
 }
 
@@ -1450,7 +1449,7 @@ void JSObject::IncrementSpillStatistics(SpillInformation* info) {
   if (HasFastProperties()) {
     info->number_of_objects_with_fast_properties_++;
     info->number_of_fast_used_fields_   += map()->NextFreePropertyIndex();
-    info->number_of_fast_unused_fields_ += map()->unused_property_fields();
+    info->number_of_fast_unused_fields_ += map()->UnusedPropertyFields();
   } else if (IsJSGlobalObject()) {
     GlobalDictionary* dict = JSGlobalObject::cast(this)->global_dictionary();
     info->number_of_slow_used_properties_ += dict->NumberOfElements();

src/objects-inl.h

@@ -1631,7 +1631,7 @@ void JSObject::InitializeBody(Map* map, int start_offset,
   int offset = start_offset;
   if (filler_value != pre_allocated_value) {
     int end_of_pre_allocated_offset =
-        size - (map->unused_property_fields() * kPointerSize);
+        size - (map->UnusedPropertyFields() * kPointerSize);
     DCHECK_LE(kHeaderSize, end_of_pre_allocated_offset);
     while (offset < end_of_pre_allocated_offset) {
       WRITE_FIELD(this, offset, pre_allocated_value);
@@ -1645,7 +1645,7 @@ void JSObject::InitializeBody(Map* map, int start_offset,
 }
 
 bool Map::TooManyFastProperties(StoreFromKeyed store_mode) const {
-  if (unused_property_fields() != 0) return false;
+  if (UnusedPropertyFields() != 0) return false;
   if (is_prototype_map()) return false;
   int minimum = store_mode == CERTAINLY_NOT_STORE_FROM_KEYED ? 128 : 12;
   int limit = Max(minimum, GetInObjectProperties());
@@ -3082,8 +3082,6 @@ void Map::set_instance_size(int value) {
 }
 
 
-void Map::clear_unused() { WRITE_BYTE_FIELD(this, kUnusedOffset, 0); }
-
 InstanceType Map::instance_type() const {
   return static_cast<InstanceType>(READ_BYTE_FIELD(this, kInstanceTypeOffset));
 }
@@ -3097,11 +3095,110 @@ int Map::unused_property_fields() const {
   return READ_BYTE_FIELD(this, kUnusedPropertyFieldsOffset);
 }
 
+int Map::UnusedPropertyFields() const {
+  VerifyUnusedPropertyFields();
+  return unused_property_fields();
+}
 
 void Map::set_unused_property_fields(int value) {
   WRITE_BYTE_FIELD(this, kUnusedPropertyFieldsOffset, Min(value, 255));
 }
 
+int Map::used_instance_size_in_words() const {
+  return READ_BYTE_FIELD(this, kUsedInstanceSizeInWordsOffset);
+}
+
+void Map::set_used_instance_size_in_words(int value) {
+  DCHECK_LE(0, value);
+  DCHECK_LE(value, 255);
+  WRITE_BYTE_FIELD(this, kUsedInstanceSizeInWordsOffset,
+                   static_cast<byte>(value));
+  VerifyUnusedPropertyFields();
+}
+
+void Map::SetInObjectUnusedPropertyFields(int value) {
+  STATIC_ASSERT(JSObject::kFieldsAdded == JSObject::kHeaderSize / kPointerSize);
+  if (!IsJSObjectMap()) {
+    DCHECK_EQ(0, value);
+    set_unused_property_fields(0);
+    set_used_instance_size_in_words(0);
+    return;
+  }
+  DCHECK_LE(0, value);
+  DCHECK_LE(value, GetInObjectProperties());
+  set_unused_property_fields(value);
+  int used_inobject_properties = GetInObjectProperties() - value;
+  set_used_instance_size_in_words(
+      GetInObjectPropertyOffset(used_inobject_properties) / kPointerSize);
+  VerifyUnusedPropertyFields();
+}
+
+void Map::SetOutOfObjectUnusedPropertyFields(int value) {
+  STATIC_ASSERT(JSObject::kFieldsAdded == JSObject::kHeaderSize / kPointerSize);
+  DCHECK_LE(0, value);
+  DCHECK_LT(value, JSObject::kFieldsAdded);
+  set_unused_property_fields(value);
+  // For out of object properties "used_instance_size_in_words" byte encodes
+  // the slack in the property array.
+  set_used_instance_size_in_words(value);
+  VerifyUnusedPropertyFields();
+}
+
+void Map::CopyUnusedPropertyFields(Map* map) {
+  set_unused_property_fields(map->unused_property_fields());
+  set_used_instance_size_in_words(map->used_instance_size_in_words());
+  VerifyUnusedPropertyFields();
+}
+
+void Map::AccountAddedPropertyField() {
+  STATIC_ASSERT(JSObject::kFieldsAdded == JSObject::kHeaderSize / kPointerSize);
+  // Update unused_property_fields.
+  int new_unused = unused_property_fields() - 1;
+  if (new_unused < 0) new_unused += JSObject::kFieldsAdded;
+  set_unused_property_fields(new_unused);
+  // Update used_instance_size_in_words.
+  int value = used_instance_size_in_words();
+  if (value >= JSObject::kFieldsAdded) {
+    if (value == instance_size() / kPointerSize) {
+      AccountAddedOutOfObjectPropertyField(0);
+    } else {
+      // The property is added in-object, so simply increment the counter.
+      set_used_instance_size_in_words(value + 1);
+    }
+  } else {
+    AccountAddedOutOfObjectPropertyField(value);
+  }
+  VerifyUnusedPropertyFields();
+}
+
+void Map::AccountAddedOutOfObjectPropertyField(int unused_in_property_array) {
+  unused_in_property_array--;
+  if (unused_in_property_array < 0) {
+    unused_in_property_array += JSObject::kFieldsAdded;
+  }
+  DCHECK_GE(unused_in_property_array, 0);
+  DCHECK_LT(unused_in_property_array, JSObject::kFieldsAdded);
+  set_used_instance_size_in_words(unused_in_property_array);
+}
+
+void Map::VerifyUnusedPropertyFields() const {
+#ifdef DEBUG
+  if (!IsJSObjectMap()) {
+    DCHECK_EQ(unused_property_fields(), used_instance_size_in_words());
+  } else {
+    int value = used_instance_size_in_words();
+    int unused;
+    if (value >= JSObject::kFieldsAdded) {
+      unused = instance_size() / kPointerSize - value;
+    } else {
+      // For out of object properties "used_instance_size_in_words" byte encodes
+      // the slack in the property array.
+      unused = value;
+    }
+    DCHECK_EQ(unused_property_fields(), unused);
+  }
+#endif
+}
 
 byte Map::bit_field() const { return READ_BYTE_FIELD(this, kBitFieldOffset); }
 
@@ -3528,8 +3625,8 @@ void Map::AppendDescriptor(Descriptor* desc) {
   }
   PropertyDetails details = desc->GetDetails();
   if (details.location() == kField) {
-    DCHECK_GT(unused_property_fields(), 0);
-    set_unused_property_fields(unused_property_fields() - 1);
+    DCHECK_GT(UnusedPropertyFields(), 0);
+    AccountAddedPropertyField();
   }
 
 // This function does not support appending double field descriptors and
@@ -3614,7 +3711,7 @@ void Map::SetConstructor(Object* constructor, WriteBarrierMode mode) {
 
 Handle<Map> Map::CopyInitialMap(Handle<Map> map) {
   return CopyInitialMap(map, map->instance_size(), map->GetInObjectProperties(),
-                        map->unused_property_fields());
+                        map->UnusedPropertyFields());
 }
 
 Object* JSBoundFunction::raw_bound_target_function() const {

src/objects-printer.cc

@@ -565,7 +565,7 @@ void Map::MapPrint(std::ostream& os) {  // NOLINT
     os << "\n - inobject properties: " << GetInObjectProperties();
   }
   os << "\n - elements kind: " << ElementsKindToString(elements_kind());
-  os << "\n - unused property fields: " << unused_property_fields();
+  os << "\n - unused property fields: " << UnusedPropertyFields();
   os << "\n - enum length: ";
   if (EnumLength() == kInvalidEnumCacheSentinel) {
     os << "invalid";

src/objects/map.h

@@ -94,7 +94,10 @@ typedef std::vector<Handle<Map>> MapHandles;
 //      |          | If Map for an Object type:                  |
 //      |          |   number of in-object properties            |
 //      +----------+---------------------------------------------+
-//      | Byte     | unused                                      |
+//      | Byte     | [used_instance_size_in_words]               |
+//      |          | For JSObject in fast mode this byte encodes |
+//      |          | the size of the object that includes only   |
+//      |          | the used property fields.                   |
 //      +----------+---------------------------------------------+
 //      | Byte     | [visitor_id]                                |
 // +----+----------+---------------------------------------------+
@@ -162,9 +165,6 @@ class Map : public HeapObject {
   inline int instance_size() const;
   inline void set_instance_size(int value);
 
-  // Only to clear an unused byte, remove once byte is used.
-  inline void clear_unused();
-
   // [inobject_properties_or_constructor_function_index]: Provides access
   // to the inobject properties in case of JSObject maps, or the constructor
   // function index in case of primitive maps.
@@ -195,6 +195,30 @@ class Map : public HeapObject {
   inline int unused_property_fields() const;
   inline void set_unused_property_fields(int value);
 
+  // This byte encodes the instance size without the slack.
+  // Let H be JSObject::kHeaderSize / kPointerSize.
+  // If value >= H then:
+  //     - all field properties are stored in the object.
+  //     - there is no property array.
+  //     - value * kPointerSize is the actual object size without the slack.
+  // Otherwise:
+  //     - there is slack in the object.
+  //     - the property array has value slack slots.
+  // Note that this encoding requires that H = JSObject::kFieldsAdded.
+  inline int used_instance_size_in_words() const;
+  inline void set_used_instance_size_in_words(int value);
+
+  inline int UnusedPropertyFields() const;
+  // Updates the counters tracking unused fields in the object.
+  inline void SetInObjectUnusedPropertyFields(int unused_property_fields);
+  // Updates the counters tracking unused fields in the property array.
+  inline void SetOutOfObjectUnusedPropertyFields(int unused_property_fields);
+  inline void CopyUnusedPropertyFields(Map* map);
+  inline void AccountAddedPropertyField();
+  inline void AccountAddedOutOfObjectPropertyField(
+      int unused_in_property_array);
+  inline void VerifyUnusedPropertyFields() const;
+
   // Bit field.
   inline byte bit_field() const;
   inline void set_bit_field(byte value);
@@ -737,9 +761,9 @@ class Map : public HeapObject {
   static const int kInObjectPropertiesOrConstructorFunctionIndexByte = 1;
   static const int kInObjectPropertiesOrConstructorFunctionIndexOffset =
       kInstanceSizesOffset + kInObjectPropertiesOrConstructorFunctionIndexByte;
-  // Note there is one byte available for use here.
-  static const int kUnusedByte = 2;
-  static const int kUnusedOffset = kInstanceSizesOffset + kUnusedByte;
+  static const int kUsedInstanceSizeInWordsByte = 2;
+  static const int kUsedInstanceSizeInWordsOffset =
+      kInstanceSizesOffset + kUsedInstanceSizeInWordsByte;
   static const int kVisitorIdByte = 3;
   static const int kVisitorIdOffset = kInstanceSizesOffset + kVisitorIdByte;
 
@@ -757,6 +781,8 @@ class Map : public HeapObject {
   static const int kInstanceTypeAndBitFieldOffset =
       kInstanceAttributesOffset + 0;
   static const int kBitField2Offset = kInstanceAttributesOffset + 2;
+  // TODO(ulan): Free this byte after unused_property_fields are computed using
+  // the used_instance_size_in_words() byte.
   static const int kUnusedPropertyFieldsOffset = kInstanceAttributesOffset + 3;
 
   STATIC_ASSERT(kInstanceTypeAndBitFieldOffset ==

src/runtime/runtime-literals.cc

@@ -373,7 +373,7 @@ struct ObjectBoilerplate {
       // TODO(cbruni): avoid making the boilerplate fast again, the clone stub
       // supports dict-mode objects directly.
       JSObject::MigrateSlowToFast(boilerplate,
-                                  boilerplate->map()->unused_property_fields(),
+                                  boilerplate->map()->UnusedPropertyFields(),
                                   "FastLiteral");
     }
     return boilerplate;