[runtime] Avoid @@isConcatSpreadable lookup for fast path Array.prototype.concat

src/isolate.cc

Index: src/isolate.cc
diff --git a/src/isolate.cc b/src/isolate.cc
index 44e5ec0658f2b424cc6c37938a1d9ba1a3bf1aee..06f09487b5afbdecc3de54c2e73f906bf9aa3117 100644
--- a/src/isolate.cc
+++ b/src/isolate.cc
@@ -2486,6 +2486,31 @@ bool Isolate::use_crankshaft() const {
          CpuFeatures::SupportsCrankshaft();
 }
 
+bool Isolate::ArrayOrObjectPrototypeIsInContext(Object* object) {

[Toon Verwaest, 2016/04/05 12:57:26]

IsArrayOrObjectPrototype

+  Object* context = heap()->native_contexts_list();
+  while (context != heap()->undefined_value()) {
+    Context* current_context = Context::cast(context);
+    if (current_context->initial_object_prototype() == object ||
+        current_context->initial_array_prototype() == object) {
+      return true;
+    }
+    context = current_context->next_context_link();
+  }
+  return false;
+}
+
+bool Isolate::IsInContextIndex(Object* object, uint32_t index) {
+  DisallowHeapAllocation no_gc;
+  Object* context = heap()->native_contexts_list();
+  while (context == heap()->undefined_value()) {
+    Context* current_context = Context::cast(context);
+    if (current_context->get(index) == object) {
+      return true;
+    }
+    context = current_context->next_context_link();
+  }
+  return false;
+}
 
 bool Isolate::IsFastArrayConstructorPrototypeChainIntact() {
   PropertyCell* no_elements_cell = heap()->array_protector();
@@ -2559,56 +2584,70 @@ bool Isolate::IsArraySpeciesLookupChainIntact() {
   // done here. In place, there are mjsunit tests harmony/array-species* which
   // ensure that behavior is correct in various invalid protector cases.
 
-  PropertyCell* species_cell = heap()->species_protector();
-  return species_cell->value()->IsSmi() &&
-         Smi::cast(species_cell->value())->value() == kArrayProtectorValid;
+  Cell* species_cell = heap()->species_protector();
+  return Smi::cast(species_cell->value())->value() == kArrayProtectorValid;
 }
 
-void Isolate::InvalidateArraySpeciesProtector() {
-  DCHECK(factory()->species_protector()->value()->IsSmi());
-  DCHECK(IsArraySpeciesLookupChainIntact());
-  PropertyCell::SetValueWithInvalidation(
-      factory()->species_protector(),
-      handle(Smi::FromInt(kArrayProtectorInvalid), this));
-  DCHECK(!IsArraySpeciesLookupChainIntact());
+bool Isolate::IsArrayIsConcatSpreadableLookupChainIntact() {
+  Cell* is_concat_spreadable_cell =
+      heap()->array_is_concat_spreadable_protector();
+  bool is_concat_spreadable_set =
+      Smi::cast(is_concat_spreadable_cell->value())->value() ==
+      kArrayProtectorInvalid;
+#ifdef DEBUG
+  Map* root_array_map = get_initial_js_array_map(GetInitialFastElementsKind());
+  if (root_array_map == NULL) {
+    // Ignore the value of is_concat_spreadable during bootstrap.
+    return !is_concat_spreadable_set;
+  }
+  Handle<Object> array_prototype(array_function()->prototype(), this);
+  Handle<Symbol> key = factory()->is_concat_spreadable_symbol();
+  Handle<Object> value;
+  LookupIterator it(array_prototype, key);
+  if (it.IsFound() && it.state() != LookupIterator::JSPROXY) {
+    MaybeHandle<Object> maybe_value = Object::GetProperty(&it);

[Toon Verwaest, 2016/04/05 12:57:26]

GetDataProperty avoids side effects. You can't just GetProperty. Not sure this
makes sense, as it won't trigger for proxies, interceptors, getters, ...

+    if (maybe_value.ToHandle(&value) && !value->IsUndefined()) {
+      // TODO(cbruni): Currently we do not revert if we unset the
+      // @@isConcatSpreadable property on Array.prototype or Object.prototype
+      // hence the reverse implication doesn't hold.
+      DCHECK(is_concat_spreadable_set);
+      return false;
+    }
+  }
+#endif  // DEBUG
+
+  return !is_concat_spreadable_set;
 }
 
 void Isolate::UpdateArrayProtectorOnSetElement(Handle<JSObject> object) {
   DisallowHeapAllocation no_gc;
-  if (IsFastArrayConstructorPrototypeChainIntact() &&
-      object->map()->is_prototype_map()) {
-    Object* context = heap()->native_contexts_list();
-    while (!context->IsUndefined()) {
-      Context* current_context = Context::cast(context);
-      if (current_context->get(Context::INITIAL_OBJECT_PROTOTYPE_INDEX) ==
-              *object ||
-          current_context->get(Context::INITIAL_ARRAY_PROTOTYPE_INDEX) ==
-              *object) {
-        CountUsage(v8::Isolate::UseCounterFeature::kArrayProtectorDirtied);
-        PropertyCell::SetValueWithInvalidation(
-            factory()->array_protector(),
-            handle(Smi::FromInt(kArrayProtectorInvalid), this));
-        break;
-      }
-      context = current_context->get(Context::NEXT_CONTEXT_LINK);
-    }
-  }
+  if (!IsFastArrayConstructorPrototypeChainIntact()) return;
+  if (!object->map()->is_prototype_map()) return;

[Toon Verwaest, 2016/04/05 12:57:26]

This check is probably faster (and will generally hit) than the one above.

+  if (!ArrayOrObjectPrototypeIsInContext(*object)) return;
+  PropertyCell::SetValueWithInvalidation(
+      factory()->array_protector(),
+      handle(Smi::FromInt(kArrayProtectorInvalid), this));
 }
 
+void Isolate::InvalidateArrayIsConcatSpreadableProtector() {
+  DCHECK(factory()->array_is_concat_spreadable_protector()->value()->IsSmi());
+  DCHECK(IsArrayIsConcatSpreadableLookupChainIntact());
+  factory()->array_is_concat_spreadable_protector()->set_value(
+      Smi::FromInt(kArrayProtectorInvalid));
+  DCHECK(!IsArrayIsConcatSpreadableLookupChainIntact());
+}
+
+void Isolate::InvalidateArraySpeciesProtector() {
+  DCHECK(factory()->species_protector()->value()->IsSmi());
+  DCHECK(IsArraySpeciesLookupChainIntact());
+  factory()->species_protector()->set_value(
+      Smi::FromInt(kArrayProtectorInvalid));
+  DCHECK(!IsArraySpeciesLookupChainIntact());
+}
 
 bool Isolate::IsAnyInitialArrayPrototype(Handle<JSArray> array) {
-  if (array->map()->is_prototype_map()) {
-    Object* context = heap()->native_contexts_list();
-    while (!context->IsUndefined()) {
-      Context* current_context = Context::cast(context);
-      if (current_context->get(Context::INITIAL_ARRAY_PROTOTYPE_INDEX) ==
-          *array) {
-        return true;
-      }
-      context = current_context->get(Context::NEXT_CONTEXT_LINK);
-    }
-  }
-  return false;
+  DisallowHeapAllocation no_gc;
+  return IsInContextIndex(*array, Context::INITIAL_ARRAY_PROTOTYPE_INDEX);
 }