Always invoke C++ ArrayPush builtin.
authorantonm@chromium.org <antonm@chromium.org@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Tue, 9 Mar 2010 15:43:04 +0000 (15:43 +0000)
committerantonm@chromium.org <antonm@chromium.org@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Tue, 9 Mar 2010 15:43:04 +0000 (15:43 +0000)
Now this builtin checks if it should go into fast case or resort to JS ArrayPush builtin.

Review URL: http://codereview.chromium.org/660298

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@4068 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

12 files changed:
src/arm/stub-cache-arm.cc
src/bootstrapper.cc
src/builtins.cc
src/contexts.h
src/ia32/stub-cache-ia32.cc
src/ic.cc
src/stub-cache.cc
src/stub-cache.h
src/top.cc
src/top.h
src/x64/stub-cache-x64.cc
test/cctest/test-serialize.cc

index 5d5b2a5..fc652c1 100644 (file)
@@ -916,18 +916,6 @@ Object* CallStubCompiler::CompileCallConstant(Object* object,
       break;
     }
 
-    case JSARRAY_HAS_FAST_ELEMENTS_CHECK:
-      CheckPrototypes(JSObject::cast(object), r1, holder, r3, r0, name, &miss);
-      // Make sure object->HasFastElements().
-      // Get the elements array of the object.
-      __ ldr(r3, FieldMemOperand(r1, JSObject::kElementsOffset));
-      // Check that the object is in fast mode (not dictionary).
-      __ ldr(r0, FieldMemOperand(r3, HeapObject::kMapOffset));
-      __ LoadRoot(ip, Heap::kFixedArrayMapRootIndex);
-      __ cmp(r0, ip);
-      __ b(ne, &miss);
-      break;
-
     default:
       UNREACHABLE();
   }
index 12efbc1..1bbfcb7 100644 (file)
@@ -245,11 +245,14 @@ class Genesis BASE_EMBEDDED {
       bool make_prototype_enumerable = false);
   void MakeFunctionInstancePrototypeWritable();
 
-  void AddSpecialFunction(Handle<JSObject> prototype,
-                          const char* name,
-                          Handle<Code> code);
+  Handle<JSFunction> MakeFunctionForBuiltin(Handle<String> name,
+                                            Handle<Code> code);
 
-  void BuildSpecialFunctionTable();
+  void OverrideWithSpecialFunction(Handle<JSObject> prototype,
+                                   const char* name,
+                                   Handle<Code> code);
+
+  void InstallSpecialFunctions();
 
   static bool CompileBuiltin(int index);
   static bool CompileNative(Vector<const char> name, Handle<String> source);
@@ -777,8 +780,6 @@ void Genesis::CreateRoots(v8::Handle<v8::ObjectTemplate> global_template,
     delegate->shared()->DontAdaptArguments();
   }
 
-  global_context()->set_special_function_table(Heap::empty_fixed_array());
-
   // Initialize the out of memory slot.
   global_context()->set_out_of_memory(Heap::false_value());
 
@@ -1457,33 +1458,35 @@ void Genesis::MakeFunctionInstancePrototypeWritable() {
 }
 
 
-void Genesis::AddSpecialFunction(Handle<JSObject> prototype,
-                                 const char* name,
-                                 Handle<Code> code) {
+Handle<JSFunction> Genesis::MakeFunctionForBuiltin(Handle<String> name,
+                                                   Handle<Code> code) {
+  Handle<JSFunction> optimized = Factory::NewFunction(name,
+                                                      JS_OBJECT_TYPE,
+                                                      JSObject::kHeaderSize,
+                                                      code,
+                                                      false);
+  optimized->shared()->DontAdaptArguments();
+  return optimized;
+}
+
+
+void Genesis::OverrideWithSpecialFunction(Handle<JSObject> prototype,
+                                          const char* name,
+                                          Handle<Code> code) {
   Handle<String> key = Factory::LookupAsciiSymbol(name);
-  Handle<Object> value = Handle<Object>(prototype->GetProperty(*key));
-  if (value->IsJSFunction()) {
-    Handle<JSFunction> optimized = Factory::NewFunction(key,
-                                                        JS_OBJECT_TYPE,
-                                                        JSObject::kHeaderSize,
-                                                        code,
-                                                        false);
-    optimized->shared()->DontAdaptArguments();
-    int len = global_context()->special_function_table()->length();
-    Handle<FixedArray> new_array = Factory::NewFixedArray(len + 3);
-    for (int index = 0; index < len; index++) {
-      new_array->set(index,
-                     global_context()->special_function_table()->get(index));
-    }
-    new_array->set(len+0, *prototype);
-    new_array->set(len+1, *value);
-    new_array->set(len+2, *optimized);
-    global_context()->set_special_function_table(*new_array);
-  }
+  Handle<Object> old_value = GetProperty(prototype, key);
+  // Check if the function is present in the first place.
+  // For example, FLAG_natives_file could affect if Array functions
+  // are installed at all.
+  if (!old_value->IsJSFunction()) return;
+  int old_length = Handle<JSFunction>::cast(old_value)->shared()->length();
+  Handle<JSFunction> optimized = MakeFunctionForBuiltin(key, code);
+  optimized->shared()->set_length(old_length);
+  SetProperty(prototype, key, optimized, NONE);
 }
 
 
-void Genesis::BuildSpecialFunctionTable() {
+void Genesis::InstallSpecialFunctions() {
   HandleScope scope;
   Handle<JSObject> global = Handle<JSObject>(global_context()->global());
   // Add special versions for some Array.prototype functions.
@@ -1501,18 +1504,24 @@ void Genesis::BuildSpecialFunctionTable() {
   } else {
     special_prototype = visible_prototype;
   }
-  AddSpecialFunction(special_prototype, "pop",
-                     Handle<Code>(Builtins::builtin(Builtins::ArrayPop)));
-  AddSpecialFunction(special_prototype, "push",
-                     Handle<Code>(Builtins::builtin(Builtins::ArrayPush)));
-  AddSpecialFunction(special_prototype, "shift",
-                     Handle<Code>(Builtins::builtin(Builtins::ArrayShift)));
-  AddSpecialFunction(special_prototype, "unshift",
-                     Handle<Code>(Builtins::builtin(Builtins::ArrayUnshift)));
-  AddSpecialFunction(special_prototype, "slice",
-                     Handle<Code>(Builtins::builtin(Builtins::ArraySlice)));
-  AddSpecialFunction(special_prototype, "splice",
-                     Handle<Code>(Builtins::builtin(Builtins::ArraySplice)));
+  OverrideWithSpecialFunction(
+      special_prototype, "pop",
+      Handle<Code>(Builtins::builtin(Builtins::ArrayPop)));
+  OverrideWithSpecialFunction(
+      special_prototype, "push",
+      Handle<Code>(Builtins::builtin(Builtins::ArrayPush)));
+  OverrideWithSpecialFunction(
+      special_prototype, "shift",
+      Handle<Code>(Builtins::builtin(Builtins::ArrayShift)));
+  OverrideWithSpecialFunction(
+      special_prototype, "unshift",
+      Handle<Code>(Builtins::builtin(Builtins::ArrayUnshift)));
+  OverrideWithSpecialFunction(
+      special_prototype, "slice",
+      Handle<Code>(Builtins::builtin(Builtins::ArraySlice)));
+  OverrideWithSpecialFunction(
+      special_prototype, "splice",
+      Handle<Code>(Builtins::builtin(Builtins::ArraySplice)));
 }
 
 
@@ -1539,7 +1548,7 @@ Genesis::Genesis(Handle<Object> global_object,
   if (!InstallNatives()) return;
 
   MakeFunctionInstancePrototypeWritable();
-  BuildSpecialFunctionTable();
+  InstallSpecialFunctions();
 
   if (!ConfigureGlobalObjects(global_template)) return;
 
index a8ba818..f5132c6 100644 (file)
@@ -319,6 +319,24 @@ static bool ArrayPrototypeHasNoElements() {
 }
 
 
+static bool IsJSArrayWithFastElements(Object* receiver,
+                                      FixedArray** elements) {
+  if (!receiver->IsJSArray()) {
+    return false;
+  }
+
+  JSArray* array = JSArray::cast(receiver);
+
+  HeapObject* elms = HeapObject::cast(array->elements());
+  if (elms->map() != Heap::fixed_array_map()) {
+    return false;
+  }
+
+  *elements = FixedArray::cast(elms);
+  return true;
+}
+
+
 static Object* CallJsBuiltin(const char* name,
                              BuiltinArguments<NO_EXTRA_ARGUMENTS> args) {
   HandleScope handleScope;
@@ -346,8 +364,12 @@ static Object* CallJsBuiltin(const char* name,
 
 
 BUILTIN(ArrayPush) {
-  JSArray* array = JSArray::cast(*args.receiver());
-  ASSERT(array->HasFastElements());
+  Object* receiver = *args.receiver();
+  FixedArray* elms = NULL;
+  if (!IsJSArrayWithFastElements(receiver, &elms)) {
+    return CallJsBuiltin("ArrayPush", args);
+  }
+  JSArray* array = JSArray::cast(receiver);
 
   int len = Smi::cast(array->length())->value();
   int to_add = args.length() - 1;
@@ -359,7 +381,6 @@ BUILTIN(ArrayPush) {
   ASSERT(to_add <= (Smi::kMaxValue - len));
 
   int new_length = len + to_add;
-  FixedArray* elms = FixedArray::cast(array->elements());
 
   if (new_length > elms->length()) {
     // New backing storage is needed.
@@ -390,14 +411,17 @@ BUILTIN(ArrayPush) {
 
 
 BUILTIN(ArrayPop) {
-  JSArray* array = JSArray::cast(*args.receiver());
-  ASSERT(array->HasFastElements());
+  Object* receiver = *args.receiver();
+  FixedArray* elms = NULL;
+  if (!IsJSArrayWithFastElements(receiver, &elms)) {
+    return CallJsBuiltin("ArrayPop", args);
+  }
+  JSArray* array = JSArray::cast(receiver);
 
   int len = Smi::cast(array->length())->value();
   if (len == 0) return Heap::undefined_value();
 
   // Get top element
-  FixedArray* elms = FixedArray::cast(array->elements());
   Object* top = elms->get(len - 1);
 
   // Set the length.
@@ -420,18 +444,18 @@ BUILTIN(ArrayPop) {
 
 
 BUILTIN(ArrayShift) {
-  if (!ArrayPrototypeHasNoElements()) {
+  Object* receiver = *args.receiver();
+  FixedArray* elms = NULL;
+  if (!IsJSArrayWithFastElements(receiver, &elms)
+      || !ArrayPrototypeHasNoElements()) {
     return CallJsBuiltin("ArrayShift", args);
   }
-
-  JSArray* array = JSArray::cast(*args.receiver());
+  JSArray* array = JSArray::cast(receiver);
   ASSERT(array->HasFastElements());
 
   int len = Smi::cast(array->length())->value();
   if (len == 0) return Heap::undefined_value();
 
-  FixedArray* elms = FixedArray::cast(array->elements());
-
   // Get first element
   Object* first = elms->get(0);
   if (first->IsTheHole()) {
@@ -451,11 +475,13 @@ BUILTIN(ArrayShift) {
 
 
 BUILTIN(ArrayUnshift) {
-  if (!ArrayPrototypeHasNoElements()) {
+  Object* receiver = *args.receiver();
+  FixedArray* elms = NULL;
+  if (!IsJSArrayWithFastElements(receiver, &elms)
+      || !ArrayPrototypeHasNoElements()) {
     return CallJsBuiltin("ArrayUnshift", args);
   }
-
-  JSArray* array = JSArray::cast(*args.receiver());
+  JSArray* array = JSArray::cast(receiver);
   ASSERT(array->HasFastElements());
 
   int len = Smi::cast(array->length())->value();
@@ -469,8 +495,6 @@ BUILTIN(ArrayUnshift) {
   // we should never hit this case.
   ASSERT(to_add <= (Smi::kMaxValue - len));
 
-  FixedArray* elms = FixedArray::cast(array->elements());
-
   if (new_length > elms->length()) {
     // New backing storage is needed.
     int capacity = new_length + (new_length >> 1) + 16;
@@ -503,11 +527,13 @@ BUILTIN(ArrayUnshift) {
 
 
 BUILTIN(ArraySlice) {
-  if (!ArrayPrototypeHasNoElements()) {
+  Object* receiver = *args.receiver();
+  FixedArray* elms = NULL;
+  if (!IsJSArrayWithFastElements(receiver, &elms)
+      || !ArrayPrototypeHasNoElements()) {
     return CallJsBuiltin("ArraySlice", args);
   }
-
-  JSArray* array = JSArray::cast(*args.receiver());
+  JSArray* array = JSArray::cast(receiver);
   ASSERT(array->HasFastElements());
 
   int len = Smi::cast(array->length())->value();
@@ -558,8 +584,6 @@ BUILTIN(ArraySlice) {
   if (result->IsFailure()) return result;
   FixedArray* result_elms = FixedArray::cast(result);
 
-  FixedArray* elms = FixedArray::cast(array->elements());
-
   AssertNoAllocation no_gc;
   CopyElements(&no_gc, result_elms, 0, elms, k, result_len);
 
@@ -573,11 +597,13 @@ BUILTIN(ArraySlice) {
 
 
 BUILTIN(ArraySplice) {
-  if (!ArrayPrototypeHasNoElements()) {
+  Object* receiver = *args.receiver();
+  FixedArray* elms = NULL;
+  if (!IsJSArrayWithFastElements(receiver, &elms)
+      || !ArrayPrototypeHasNoElements()) {
     return CallJsBuiltin("ArraySplice", args);
   }
-
-  JSArray* array = JSArray::cast(*args.receiver());
+  JSArray* array = JSArray::cast(receiver);
   ASSERT(array->HasFastElements());
 
   int len = Smi::cast(array->length())->value();
@@ -618,8 +644,6 @@ BUILTIN(ArraySplice) {
   }
   int actual_delete_count = Min(Max(delete_count, 0), len - actual_start);
 
-  FixedArray* elms = FixedArray::cast(array->elements());
-
   JSArray* result_array = NULL;
   if (actual_delete_count == 0) {
     Object* result = AllocateEmptyJSArray();
index 98ebc47..4997741 100644 (file)
@@ -50,12 +50,6 @@ enum ContextLookupFlags {
 // must always be allocated via Heap::AllocateContext() or
 // Factory::NewContext.
 
-// Comment for special_function_table:
-// Table for providing optimized/specialized functions.
-// The array contains triplets [object, general_function, optimized_function].
-// Primarily added to support built-in optimized variants of
-// Array.prototype.{push,pop}.
-
 #define GLOBAL_CONTEXT_FIELDS(V) \
   V(GLOBAL_PROXY_INDEX, JSObject, global_proxy_object) \
   V(SECURITY_TOKEN_INDEX, Object, security_token) \
@@ -82,7 +76,6 @@ enum ContextLookupFlags {
   V(FUNCTION_MAP_INDEX, Map, function_map) \
   V(FUNCTION_INSTANCE_MAP_INDEX, Map, function_instance_map) \
   V(JS_ARRAY_MAP_INDEX, Map, js_array_map)\
-  V(SPECIAL_FUNCTION_TABLE_INDEX, FixedArray, special_function_table) \
   V(ARGUMENTS_BOILERPLATE_INDEX, JSObject, arguments_boilerplate) \
   V(MESSAGE_LISTENERS_INDEX, JSObject, message_listeners) \
   V(MAKE_MESSAGE_FUN_INDEX, JSFunction, make_message_fun) \
@@ -206,7 +199,6 @@ class Context: public FixedArray {
     GLOBAL_EVAL_FUN_INDEX,
     INSTANTIATE_FUN_INDEX,
     CONFIGURE_INSTANCE_FUN_INDEX,
-    SPECIAL_FUNCTION_TABLE_INDEX,
     MESSAGE_LISTENERS_INDEX,
     MAKE_MESSAGE_FUN_INDEX,
     GET_STACK_TRACE_LINE_INDEX,
index a7e9a69..697a71f 100644 (file)
@@ -698,8 +698,7 @@ class CallInterceptorCompiler BASE_EMBEDDED {
 
     CallOptimization optimization(lookup);
 
-    if (optimization.is_constant_call() &&
-        !Top::CanHaveSpecialFunctions(holder)) {
+    if (optimization.is_constant_call()) {
       CompileCacheable(masm,
                        object,
                        receiver,
@@ -1333,18 +1332,6 @@ Object* CallStubCompiler::CompileCallConstant(Object* object,
       break;
     }
 
-    case JSARRAY_HAS_FAST_ELEMENTS_CHECK:
-      CheckPrototypes(JSObject::cast(object), edx, holder,
-                      ebx, eax, name, &miss);
-      // Make sure object->HasFastElements().
-      // Get the elements array of the object.
-      __ mov(ebx, FieldOperand(edx, JSObject::kElementsOffset));
-      // Check that the object is in fast mode (not dictionary).
-      __ cmp(FieldOperand(ebx, HeapObject::kMapOffset),
-             Immediate(Factory::fixed_array_map()));
-      __ j(not_equal, &miss, not_taken);
-      break;
-
     default:
       UNREACHABLE();
   }
index f82e61e..e87fd75 100644 (file)
--- a/src/ic.cc
+++ b/src/ic.cc
@@ -458,17 +458,6 @@ Object* CallIC::LoadFunction(State state,
   ASSERT(result != Heap::the_hole_value());
 
   if (result->IsJSFunction()) {
-    // Check if there is an optimized (builtin) version of the function.
-    // Ignored this will degrade performance for some Array functions.
-    // Please note we only return the optimized function iff
-    // the JSObject has FastElements.
-    if (object->IsJSObject() && JSObject::cast(*object)->HasFastElements()) {
-      Object* opt = Top::LookupSpecialFunction(JSObject::cast(*object),
-                                               lookup.holder(),
-                                               JSFunction::cast(result));
-      if (opt->IsJSFunction()) return opt;
-    }
-
 #ifdef ENABLE_DEBUGGER_SUPPORT
     // Handle stepping into a function if step into is active.
     if (Debug::StepInActive()) {
index 577c2d7..c942d9d 100644 (file)
@@ -435,14 +435,6 @@ Object* StubCache::ComputeCallConstant(int argc,
                                     argc);
   Object* code = map->FindInCodeCache(name, flags);
   if (code->IsUndefined()) {
-    if (object->IsJSObject()) {
-      Object* opt =
-          Top::LookupSpecialFunction(JSObject::cast(object), holder, function);
-      if (opt->IsJSFunction()) {
-        check = StubCompiler::JSARRAY_HAS_FAST_ELEMENTS_CHECK;
-        function = JSFunction::cast(opt);
-      }
-    }
     // If the function hasn't been compiled yet, we cannot do it now
     // because it may cause GC. To avoid this issue, we return an
     // internal error which will make sure we do not update any
index 43354db..278b05c 100644 (file)
@@ -326,8 +326,7 @@ class StubCompiler BASE_EMBEDDED {
     RECEIVER_MAP_CHECK,
     STRING_CHECK,
     NUMBER_CHECK,
-    BOOLEAN_CHECK,
-    JSARRAY_HAS_FAST_ELEMENTS_CHECK
+    BOOLEAN_CHECK
   };
 
   StubCompiler() : scope_(), masm_(NULL, 256), failure_(NULL) { }
index 7c8a1c9..850bf27 100644 (file)
@@ -950,27 +950,6 @@ Handle<Context> Top::GetCallingGlobalContext() {
 }
 
 
-bool Top::CanHaveSpecialFunctions(JSObject* object) {
-  return object->IsJSArray();
-}
-
-
-Object* Top::LookupSpecialFunction(JSObject* receiver,
-                                   JSObject* prototype,
-                                   JSFunction* function) {
-  if (CanHaveSpecialFunctions(receiver)) {
-    FixedArray* table = context()->global_context()->special_function_table();
-    for (int index = 0; index < table->length(); index +=3) {
-      if ((prototype == table->get(index)) &&
-          (function == table->get(index+1))) {
-        return table->get(index+2);
-      }
-    }
-  }
-  return Heap::undefined_value();
-}
-
-
 char* Top::ArchiveThread(char* to) {
   memcpy(to, reinterpret_cast<char*>(&thread_local_), sizeof(thread_local_));
   InitializeThreadLocal();
index ddc73ba..e20a2a0 100644 (file)
--- a/src/top.h
+++ b/src/top.h
@@ -342,11 +342,6 @@ class Top {
     return Handle<JSBuiltinsObject>(thread_local_.context_->builtins());
   }
 
-  static bool CanHaveSpecialFunctions(JSObject* object);
-  static Object* LookupSpecialFunction(JSObject* receiver,
-                                       JSObject* prototype,
-                                       JSFunction* value);
-
   static void RegisterTryCatchHandler(v8::TryCatch* that);
   static void UnregisterTryCatchHandler(v8::TryCatch* that);
 
index 5d73b5f..1517fd3 100644 (file)
@@ -759,18 +759,6 @@ Object* CallStubCompiler::CompileCallConstant(Object* object,
       break;
     }
 
-    case JSARRAY_HAS_FAST_ELEMENTS_CHECK:
-      CheckPrototypes(JSObject::cast(object), rdx, holder,
-                      rbx, rax, name, &miss);
-      // Make sure object->HasFastElements().
-      // Get the elements array of the object.
-      __ movq(rbx, FieldOperand(rdx, JSObject::kElementsOffset));
-      // Check that the object is in fast mode (not dictionary).
-      __ Cmp(FieldOperand(rbx, HeapObject::kMapOffset),
-             Factory::fixed_array_map());
-      __ j(not_equal, &miss);
-      break;
-
     default:
       UNREACHABLE();
   }
index 1283dfd..4308ff5 100644 (file)
@@ -283,7 +283,6 @@ static void SanityCheck() {
 #endif
   CHECK(Top::global()->IsJSObject());
   CHECK(Top::global_context()->IsContext());
-  CHECK(Top::special_function_table()->IsFixedArray());
   CHECK(Heap::symbol_table()->IsSymbolTable());
   CHECK(!Factory::LookupAsciiSymbol("Empty")->IsFailure());
 }