[compiler] Perform Map::GetConstructor concurrently

We can ensure that the constructor is set before the map is set on the
JSObject. Setting the constructor remains non-atomic.

Bug: v8:7790
Change-Id: Ie65519f61e29c9bed89bf09f582aa8bd39de1b03
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2761199Reviewed-by: Georg Neis <neis@chromium.org>
Commit-Queue: Santiago Aboy Solanes <solanes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#73460}

src/compiler/js-heap-broker.cc

@@ -3482,7 +3482,7 @@ BIMODAL_ACCESSOR_C(Map, int, NextFreePropertyIndex)
 BIMODAL_ACCESSOR_C(Map, int, UnusedPropertyFields)
 BIMODAL_ACCESSOR(Map, HeapObject, prototype)
 BIMODAL_ACCESSOR_C(Map, InstanceType, instance_type)
-BIMODAL_ACCESSOR(Map, Object, GetConstructor)
+BIMODAL_ACCESSOR_WITH_FLAG(Map, Object, GetConstructor)
 BIMODAL_ACCESSOR_WITH_FLAG(Map, HeapObject, GetBackPointer)
 BIMODAL_ACCESSOR_C(Map, bool, is_abandoned_prototype_map)
 

src/objects/js-function.cc

@@ -463,9 +463,9 @@ void JSFunction::SetPrototype(Handle<JSFunction> function,
     Handle<Map> new_map =
         Map::Copy(isolate, handle(function->map(), isolate), "SetPrototype");
 
-    JSObject::MigrateToMap(isolate, function, new_map);
     new_map->SetConstructor(*value);
     new_map->set_has_non_instance_prototype(true);
+    JSObject::MigrateToMap(isolate, function, new_map);
 
     FunctionKind kind = function->shared().kind();
     Handle<Context> native_context(function->context().native_context(),
@@ -488,12 +488,18 @@ void JSFunction::SetPrototype(Handle<JSFunction> function,
 
 void JSFunction::SetInitialMap(Handle<JSFunction> function, Handle<Map> map,
                                Handle<HeapObject> prototype) {
+  SetInitialMap(function, map, prototype, function);
+}
+
+void JSFunction::SetInitialMap(Handle<JSFunction> function, Handle<Map> map,
+                               Handle<HeapObject> prototype,
+                               Handle<JSFunction> constructor) {
   Isolate* isolate = function->GetIsolate();
   if (map->prototype() != *prototype) {
     Map::SetPrototype(isolate, map, prototype);
   }
+  map->SetConstructor(*constructor);
   function->set_prototype_or_initial_map(*map);
-  map->SetConstructor(*function);
   if (FLAG_log_maps) {
     LOG(isolate, MapEvent("InitialMap", Handle<Map>(), map, "",
                           SharedFunctionInfo::DebugName(
@@ -719,9 +725,8 @@ bool FastInitializeDerivedMap(Isolate* isolate, Handle<JSFunction> new_target,
                           in_object_properties, unused_property_fields);
   map->set_new_target_is_base(false);
   Handle<HeapObject> prototype(new_target->instance_prototype(), isolate);
-  JSFunction::SetInitialMap(new_target, map, prototype);
+  JSFunction::SetInitialMap(new_target, map, prototype, constructor);
   DCHECK(new_target->instance_prototype().IsJSReceiver());
-  map->SetConstructor(*constructor);
   map->set_construction_counter(Map::kNoSlackTracking);
   map->StartInobjectSlackTracking();
   return true;

src/objects/js-function.h

@@ -223,6 +223,9 @@ class JSFunction : public JSFunctionOrBoundFunction {
 
   static void SetInitialMap(Handle<JSFunction> function, Handle<Map> map,
                             Handle<HeapObject> prototype);
+  static void SetInitialMap(Handle<JSFunction> function, Handle<Map> map,
+                            Handle<HeapObject> prototype,
+                            Handle<JSFunction> constructor);
   DECL_GETTER(has_initial_map, bool)
   V8_EXPORT_PRIVATE static void EnsureHasInitialMap(
       Handle<JSFunction> function);

src/objects/js-objects.cc

@@ -4420,7 +4420,20 @@ void JSObject::OptimizeAsPrototype(Handle<JSObject> object,
   } else {
     Handle<Map> new_map =
         Map::Copy(isolate, handle(object->map(), isolate), "CopyAsPrototype");
+    new_map->set_is_prototype_map(true);
 
+    // Replace the pointer to the exact constructor with the Object function
+    // from the same context if undetectable from JS. This is to avoid keeping
+    // memory alive unnecessarily.
+    Object maybe_constructor = new_map->GetConstructor();
+    if (maybe_constructor.IsJSFunction()) {
+      JSFunction constructor = JSFunction::cast(maybe_constructor);
+      if (!constructor.shared().IsApiFunction()) {
+        Context context = constructor.context().native_context();
+        JSFunction object_function = context.object_function();
+        new_map->SetConstructor(object_function);
+      }
+    }
     JSObject::MigrateToMap(isolate, object, new_map);
 
     if (V8_DICT_PROPERTY_CONST_TRACKING_BOOL && !object->HasFastProperties()) {
@@ -4443,21 +4456,6 @@ void JSObject::OptimizeAsPrototype(Handle<JSObject> object,
         make_constant(object->property_dictionary());
       }
     }
-
-    object->map().set_is_prototype_map(true);
-
-    // Replace the pointer to the exact constructor with the Object function
-    // from the same context if undetectable from JS. This is to avoid keeping
-    // memory alive unnecessarily.
-    Object maybe_constructor = object->map().GetConstructor();
-    if (maybe_constructor.IsJSFunction()) {
-      JSFunction constructor = JSFunction::cast(maybe_constructor);
-      if (!constructor.shared().IsApiFunction()) {
-        Context context = constructor.context().native_context();
-        JSFunction object_function = context.object_function();
-        object->map().SetConstructor(object_function);
-      }
-    }
   }
 #ifdef DEBUG
   bool should_be_dictionary = V8_DICT_PROPERTY_CONST_TRACKING_BOOL &&

src/objects/map-inl.h

@@ -657,12 +657,14 @@ void Map::AppendDescriptor(Isolate* isolate, Descriptor* desc) {
 #endif
 }
 
+bool Map::ConcurrentIsMap(IsolateRoot isolate, const Object& object) const {
+  return object.IsHeapObject() && HeapObject::cast(object).map(isolate) ==
+                                      GetReadOnlyRoots(isolate).meta_map();
+}
+
 DEF_GETTER(Map, GetBackPointer, HeapObject) {
   Object object = constructor_or_back_pointer(isolate);
-  // This is the equivalent of IsMap() but avoids reading the instance type so
-  // it can be used concurrently without acquire load.
-  if (object.IsHeapObject() && HeapObject::cast(object).map(isolate) ==
-                                   GetReadOnlyRoots(isolate).meta_map()) {
+  if (ConcurrentIsMap(isolate, object)) {
     return Map::cast(object);
   }
   // Can't use ReadOnlyRoots(isolate) as this isolate could be produced by
@@ -674,8 +676,7 @@ void Map::SetBackPointer(HeapObject value, WriteBarrierMode mode) {
   CHECK_GE(instance_type(), FIRST_JS_RECEIVER_TYPE);
   CHECK(value.IsMap());
   CHECK(GetBackPointer().IsUndefined());
-  CHECK_IMPLIES(value.IsMap(), Map::cast(value).GetConstructor() ==
-                                   constructor_or_back_pointer());
+  CHECK_EQ(Map::cast(value).GetConstructor(), constructor_or_back_pointer());
   set_constructor_or_back_pointer(value, mode);
 }
 
@@ -710,7 +711,7 @@ bool Map::IsPrototypeValidityCellValid() const {
 DEF_GETTER(Map, GetConstructor, Object) {
   Object maybe_constructor = constructor_or_back_pointer(isolate);
   // Follow any back pointers.
-  while (maybe_constructor.IsMap(isolate)) {
+  while (ConcurrentIsMap(isolate, maybe_constructor)) {
     maybe_constructor =
         Map::cast(maybe_constructor).constructor_or_back_pointer(isolate);
   }

src/objects/map.h

@@ -941,6 +941,11 @@ class Map : public HeapObject {
       MaybeHandle<Object> old_value, MaybeHandle<FieldType> new_field_type,
       MaybeHandle<Object> new_value);
 
+  // This is the equivalent of IsMap() but avoids reading the instance type so
+  // it can be used concurrently without acquire load.
+  V8_INLINE bool ConcurrentIsMap(IsolateRoot isolate,
+                                 const Object& object) const;
+
   // Use the high-level instance_descriptors/SetInstanceDescriptors instead.
   DECL_RELEASE_SETTER(instance_descriptors, DescriptorArray)