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

src/objects.cc

 // Copyright 2013 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "src/objects.h"
 
 #include <cmath>
 #include <iomanip>
 #include <sstream>
 
@@ skipping 3551 matching lines @@
   RETURN_VALUE_IF_SCHEDULED_EXCEPTION(it->isolate(), Nothing<bool>());
   if (result.IsEmpty()) return Just(false);
 #ifdef DEBUG
   Handle<Object> result_internal = v8::Utils::OpenHandle(*result);
   result_internal->VerifyApiCallResultType();
 #endif
   return Just(true);
 }
 
 
-MaybeHandle<Object> Object::SetProperty(Handle<Object> object,
-                                        Handle<Name> name, Handle<Object> value,
-                                        LanguageMode language_mode,
-                                        StoreFromKeyed store_mode) {
-  LookupIterator it(object, name);
-  return SetProperty(&it, value, language_mode, store_mode);
-}
-
-
 Maybe<bool> Object::SetPropertyInternal(
     LookupIterator* it, Handle<Object> value, LanguageMode language_mode,
     ShouldThrow should_throw, StoreFromKeyed store_mode, bool* found) {
   // Make sure that the top context does not change when doing callbacks or
   // interceptor calls.
   AssertNoContextChange ncc(it->isolate());
 
   *found = true;
 
   bool done = false;
@@ skipping 87 matching lines @@
   if (it->GetReceiver()->IsJSGlobalObject() && is_strict(language_mode)) {
     RETURN_FAILURE(it->isolate(), should_throw,
                    NewReferenceError(MessageTemplate::kNotDefined, it->name()));
   }
 
   *found = false;
   return Nothing<bool>();
 }
 
 
+MaybeHandle<Object> Object::SetProperty(Handle<Object> object,
+                                        Handle<Name> name, Handle<Object> value,
+                                        LanguageMode language_mode,
+                                        StoreFromKeyed store_mode) {
+  LookupIterator it(object, name);
+  return SetProperty(&it, value, language_mode, store_mode);
+}
+
+
 MaybeHandle<Object> Object::SetProperty(LookupIterator* it,
                                         Handle<Object> value,
                                         LanguageMode language_mode,
                                         StoreFromKeyed store_mode) {
   MAYBE_RETURN_NULL(
       SetProperty(it, value, language_mode, THROW_ON_ERROR, store_mode));
   return value;
 }
 
 
@@ skipping 199 matching lines @@
 
       // We have to recheck the length. However, it can only change if the
       // underlying buffer was neutered, so just check that.
       if (Handle<JSArrayBufferView>::cast(receiver)->WasNeutered()) {
         return Just(true);
         // TODO(neis): According to the spec, this should throw a TypeError.
       }
     }
   }
 
+  //  it->isolate()->UpdateArrayIsConcatSpreadableProtectorOnAddProperty(
+  //     receiver, it->GetName());

[Toon Verwaest, 2015/11/02 12:32:46]

stray change?

[Camillo Bruni, 2015/11/03 14:49:25]

indeed.

+
   // Possibly migrate to the most up-to-date map that will be able to store
   // |value| under it->name().
   it->PrepareForDataProperty(to_assign);
 
   // Write the property value.
   it->WriteDataValue(to_assign);
 
   // Send the change record if there are observers.
   if (is_observed && !value->SameValue(*maybe_old.ToHandleChecked())) {
     RETURN_ON_EXCEPTION_VALUE(
@@ skipping 112 matching lines @@
       if (FLAG_trace_js_array_abuse && !array->HasFixedTypedArrayElements()) {
         CheckArrayAbuse(array, "elements write", it->index(), false);
       }
     }
 
     Maybe<bool> result = JSObject::AddDataElement(receiver, it->index(), value,
                                                   attributes, should_throw);
     JSObject::ValidateElements(receiver);
     return result;
   } else {
+    it->isolate()->UpdateArrayIsConcatSpreadableProtectorOnAddProperty(

[Toon Verwaest, 2015/11/02 12:32:45]

I don't think all stores go through here. Generic keyed stores to dictionary
mode receivers probably don't.

[Camillo Bruni, 2015/11/03 14:49:25]

I added another test case, if I understood it correctly we should be in exactly
this case (generic IC with dict mode receiver) and we still end up here.

+        receiver, it->name());
     // Migrate to the most up-to-date map that will be able to store |value|
     // under it->name() with |attributes|.
     it->PrepareTransitionToDataProperty(value, attributes, store_mode);
     DCHECK_EQ(LookupIterator::TRANSITION, it->state());
     it->ApplyTransitionToDataProperty();
 
     // TODO(verwaest): Encapsulate dictionary handling better.
     if (receiver->map()->is_dictionary_map()) {
       // TODO(verwaest): Probably should ensure this is done beforehand.
       it->InternalizeName();
@@ skipping 13798 matching lines @@
   if (cell->value() != *new_value) {
     cell->set_value(*new_value);
     Isolate* isolate = cell->GetIsolate();
     cell->dependent_code()->DeoptimizeDependentCodeGroup(
         isolate, DependentCode::kPropertyCellChangedGroup);
   }
 }
 
 }  // namespace internal
 }  // namespace v8