[runtime] Refactor initialization of in-object property count of a map.

This patch moves initialization of inobject_properties and
unused_property_fields of a map to the construction time of the map.

Map::AppendDescriptor now properly decrements unused_property_fields
and thus maintains the invariant for property field counters.

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

src/factory.cc

@@ -1285,13 +1285,13 @@ Handle<AllocationSite> Factory::NewAllocationSite() {
   return site;
 }
 
-
-Handle<Map> Factory::NewMap(InstanceType type,
-                            int instance_size,
-                            ElementsKind elements_kind) {
+Handle<Map> Factory::NewMap(InstanceType type, int instance_size,
+                            ElementsKind elements_kind,
+                            int inobject_properties) {
   CALL_HEAP_FUNCTION(
       isolate(),
-      isolate()->heap()->AllocateMap(type, instance_size, elements_kind),
+      isolate()->heap()->AllocateMap(type, instance_size, elements_kind,
+                                     inobject_properties),
       Map);
 }
 
@@ -1618,6 +1618,7 @@ Handle<JSFunction> Factory::NewFunction(Handle<String> name, Handle<Code> code,
 Handle<JSFunction> Factory::NewFunction(Handle<String> name, Handle<Code> code,
                                         Handle<Object> prototype,
                                         InstanceType type, int instance_size,
+                                        int inobject_properties,
                                         LanguageMode language_mode,
                                         MutableMode prototype_mutability) {
   // Allocate the function
@@ -1626,7 +1627,8 @@ Handle<JSFunction> Factory::NewFunction(Handle<String> name, Handle<Code> code,
 
   ElementsKind elements_kind =
       type == JS_ARRAY_TYPE ? PACKED_SMI_ELEMENTS : HOLEY_SMI_ELEMENTS;
-  Handle<Map> initial_map = NewMap(type, instance_size, elements_kind);
+  Handle<Map> initial_map =
+      NewMap(type, instance_size, elements_kind, inobject_properties);
   // TODO(littledan): Why do we have this is_generator test when
   // NewFunctionPrototype already handles finding an appropriately
   // shared prototype?
@@ -1639,14 +1641,12 @@ Handle<JSFunction> Factory::NewFunction(Handle<String> name, Handle<Code> code,
   return function;
 }
 
-
-Handle<JSFunction> Factory::NewFunction(Handle<String> name,
-                                        Handle<Code> code,
-                                        InstanceType type,
-                                        int instance_size) {
+Handle<JSFunction> Factory::NewFunction(Handle<String> name, Handle<Code> code,
+                                        InstanceType type, int instance_size,
+                                        int inobject_properties) {
   DCHECK(isolate()->bootstrapper()->IsActive());
   return NewFunction(name, code, the_hole_value(), type, instance_size,
-                     LanguageMode::kStrict);
+                     inobject_properties, LanguageMode::kStrict);
 }
 
 
@@ -2946,7 +2946,8 @@ Handle<Map> Factory::CreateSloppyFunctionMap(
   if (IsFunctionModeWithName(function_mode)) ++inobject_properties_count;
 
   Handle<Map> map = NewMap(
-      JS_FUNCTION_TYPE, header_size + inobject_properties_count * kPointerSize);
+      JS_FUNCTION_TYPE, header_size + inobject_properties_count * kPointerSize,
+      TERMINAL_FAST_ELEMENTS_KIND, inobject_properties_count);
   map->set_has_prototype_slot(has_prototype);
   map->set_is_constructor(has_prototype);
   map->set_is_callable();
@@ -2954,7 +2955,6 @@ Handle<Map> Factory::CreateSloppyFunctionMap(
   if (maybe_empty_function.ToHandle(&empty_function)) {
     Map::SetPrototype(map, empty_function);
   }
-  map->SetInObjectProperties(inobject_properties_count);
 
   //
   // Setup descriptors array.
@@ -3035,12 +3035,12 @@ Handle<Map> Factory::CreateStrictFunctionMap(
                           inobject_properties_count;
 
   Handle<Map> map = NewMap(
-      JS_FUNCTION_TYPE, header_size + inobject_properties_count * kPointerSize);
+      JS_FUNCTION_TYPE, header_size + inobject_properties_count * kPointerSize,
+      TERMINAL_FAST_ELEMENTS_KIND, inobject_properties_count);
   map->set_has_prototype_slot(has_prototype);
   map->set_is_constructor(has_prototype);
   map->set_is_callable();
   Map::SetPrototype(map, empty_function);
-  map->SetInObjectProperties(inobject_properties_count);
 
   //
   // Setup descriptors array.

src/factory.h

@@ -418,7 +418,8 @@ class V8_EXPORT_PRIVATE Factory final {
   Handle<AllocationSite> NewAllocationSite();
 
   Handle<Map> NewMap(InstanceType type, int instance_size,
-                     ElementsKind elements_kind = TERMINAL_FAST_ELEMENTS_KIND);
+                     ElementsKind elements_kind = TERMINAL_FAST_ELEMENTS_KIND,
+                     int inobject_properties = 0);
 
   Handle<HeapObject> NewFillerObject(int size,
                                      bool double_align,
@@ -650,13 +651,12 @@ class V8_EXPORT_PRIVATE Factory final {
 
   Handle<JSFunction> NewFunction(
       Handle<String> name, Handle<Code> code, Handle<Object> prototype,
-      InstanceType type, int instance_size,
+      InstanceType type, int instance_size, int inobject_properties,
       LanguageMode language_mode = LanguageMode::kSloppy,
       MutableMode prototype_mutability = MUTABLE);
-  Handle<JSFunction> NewFunction(Handle<String> name,
-                                 Handle<Code> code,
-                                 InstanceType type,
-                                 int instance_size);
+  Handle<JSFunction> NewFunction(Handle<String> name, Handle<Code> code,
+                                 InstanceType type, int instance_size,
+                                 int inobject_properties);
   Handle<JSFunction> NewFunction(Handle<Map> map, Handle<String> name,
                                  MaybeHandle<Code> maybe_code);
 

src/heap/heap.cc

@@ -2338,10 +2338,10 @@ AllocationResult Heap::AllocatePartialMap(InstanceType instance_type,
   return map;
 }
 
-
 AllocationResult Heap::AllocateMap(InstanceType instance_type,
                                    int instance_size,
-                                   ElementsKind elements_kind) {
+                                   ElementsKind elements_kind,
+                                   int inobject_properties) {
   HeapObject* result = nullptr;
   AllocationResult allocation = AllocateRaw(Map::kSize, MAP_SPACE);
   if (!allocation.To(&result)) return allocation;
@@ -2354,12 +2354,13 @@ AllocationResult Heap::AllocateMap(InstanceType instance_type,
   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(0);
+  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(0);
+  map->set_unused_property_fields(inobject_properties);
   map->set_instance_descriptors(empty_descriptor_array());
   if (FLAG_unbox_double_fields) {
     map->set_layout_descriptor(LayoutDescriptor::FastPointerLayout());

src/heap/heap.h

@@ -1967,7 +1967,8 @@ class Heap {
   // Allocates a JS Map in the heap.
   MUST_USE_RESULT AllocationResult
   AllocateMap(InstanceType instance_type, int instance_size,
-              ElementsKind elements_kind = TERMINAL_FAST_ELEMENTS_KIND);
+              ElementsKind elements_kind = TERMINAL_FAST_ELEMENTS_KIND,
+              int inobject_properties = 0);
 
   // Allocates and initializes a new JavaScript object based on a
   // constructor.

src/objects-inl.h

@@ -3526,11 +3526,15 @@ void Map::AppendDescriptor(Descriptor* desc) {
   if (desc->GetKey()->IsInterestingSymbol()) {
     set_may_have_interesting_symbols(true);
   }
+  PropertyDetails details = desc->GetDetails();
+  if (details.location() == kField) {
+    DCHECK_GT(unused_property_fields(), 0);
+    set_unused_property_fields(unused_property_fields() - 1);
+  }
 
 // This function does not support appending double field descriptors and
 // it should never try to (otherwise, layout descriptor must be updated too).
 #ifdef DEBUG
-  PropertyDetails details = desc->GetDetails();
   DCHECK(details.location() != kField || !details.representation().IsDouble());
 #endif
 }

src/objects.cc

@@ -9001,11 +9001,12 @@ Object* JSObject::SlowReverseLookup(Object* value) {
   }
 }
 
-
-Handle<Map> Map::RawCopy(Handle<Map> map, int instance_size) {
+Handle<Map> Map::RawCopy(Handle<Map> map, int instance_size,
+                         int inobject_properties) {
   Isolate* isolate = map->GetIsolate();
-  Handle<Map> result =
-      isolate->factory()->NewMap(map->instance_type(), instance_size);
+  Handle<Map> result = isolate->factory()->NewMap(
+      map->instance_type(), instance_size, TERMINAL_FAST_ELEMENTS_KIND,
+      inobject_properties);
   Handle<Object> prototype(map->prototype(), isolate);
   Map::SetPrototype(result, prototype);
   result->set_constructor_or_backpointer(map->GetConstructor());
@@ -9097,12 +9098,12 @@ Handle<Map> Map::CopyNormalized(Handle<Map> map,
     new_instance_size -= map->GetInObjectProperties() * kPointerSize;
   }
 
-  Handle<Map> result = RawCopy(map, new_instance_size);
-
-  if (mode != CLEAR_INOBJECT_PROPERTIES) {
-    result->SetInObjectProperties(map->GetInObjectProperties());
-  }
-
+  Handle<Map> result = RawCopy(
+      map, new_instance_size,
+      mode == CLEAR_INOBJECT_PROPERTIES ? 0 : map->GetInObjectProperties());
+  // Clear the unused_property_fields explicitly as this field should not
+  // be accessed for normalized maps.
+  result->set_unused_property_fields(0);
   result->set_dictionary_map(true);
   result->set_migration_target(false);
   result->set_may_have_interesting_symbols(true);
@@ -9165,13 +9166,12 @@ Handle<Map> Map::CopyInitialMapNormalized(Handle<Map> map,
 
 // static
 Handle<Map> Map::CopyInitialMap(Handle<Map> map, int instance_size,
-                                int in_object_properties,
+                                int inobject_properties,
                                 int unused_property_fields) {
   EnsureInitialMap(map);
-  Handle<Map> result = RawCopy(map, instance_size);
+  Handle<Map> result = RawCopy(map, instance_size, inobject_properties);
 
   // Please note instance_type and instance_size are set when allocated.
-  result->SetInObjectProperties(in_object_properties);
   result->set_unused_property_fields(unused_property_fields);
 
   int number_of_own_descriptors = map->NumberOfOwnDescriptors();
@@ -9181,8 +9181,8 @@ Handle<Map> Map::CopyInitialMap(Handle<Map> map, int instance_size,
                               map->GetLayoutDescriptor());
     result->SetNumberOfOwnDescriptors(number_of_own_descriptors);
 
-    DCHECK_EQ(result->NumberOfFields(),
-              in_object_properties - unused_property_fields);
+    DCHECK_EQ(result->NumberOfFields(), result->GetInObjectProperties() -
+                                            result->unused_property_fields());
   }
 
   return result;
@@ -9190,11 +9190,12 @@ Handle<Map> Map::CopyInitialMap(Handle<Map> map, int instance_size,
 
 
 Handle<Map> Map::CopyDropDescriptors(Handle<Map> map) {
-  Handle<Map> result = RawCopy(map, map->instance_size());
+  Handle<Map> result =
+      RawCopy(map, map->instance_size(),
+              map->IsJSObjectMap() ? map->GetInObjectProperties() : 0);
 
   // Please note instance_type and instance_size are set when allocated.
   if (map->IsJSObjectMap()) {
-    result->SetInObjectProperties(map->GetInObjectProperties());
     result->set_unused_property_fields(map->unused_property_fields());
   }
   map->NotifyLeafMapLayoutChange();
@@ -12907,11 +12908,13 @@ void JSFunction::EnsureHasInitialMap(Handle<JSFunction> function) {
   }
 
   int instance_size;
-  int in_object_properties;
+  int inobject_properties;
   CalculateInstanceSizeHelper(instance_type, false, 0, expected_nof_properties,
-                              &instance_size, &in_object_properties);
+                              &instance_size, &inobject_properties);
 
-  Handle<Map> map = isolate->factory()->NewMap(instance_type, instance_size);
+  Handle<Map> map = isolate->factory()->NewMap(instance_type, instance_size,
+                                               TERMINAL_FAST_ELEMENTS_KIND,
+                                               inobject_properties);
 
   // Fetch or allocate prototype.
   Handle<Object> prototype;
@@ -12920,8 +12923,6 @@ void JSFunction::EnsureHasInitialMap(Handle<JSFunction> function) {
   } else {
     prototype = isolate->factory()->NewFunctionPrototype(function);
   }
-  map->SetInObjectProperties(in_object_properties);
-  map->set_unused_property_fields(in_object_properties);
   DCHECK(map->has_fast_object_elements());
 
   // Finally link initial map and constructor function.

src/objects/map.h

@@ -843,7 +843,8 @@ class Map : public HeapObject {
 
   bool EquivalentToForTransition(const Map* other) const;
   bool EquivalentToForElementsKindTransition(const Map* other) const;
-  static Handle<Map> RawCopy(Handle<Map> map, int instance_size);
+  static Handle<Map> RawCopy(Handle<Map> map, int instance_size,
+                             int inobject_properties);
   static Handle<Map> ShareDescriptor(Handle<Map> map,
                                      Handle<DescriptorArray> descriptors,
                                      Descriptor* descriptor);