Improve performance of allocating closures for nested

functions by allocating them in new space without
entering the runtime system.
Review URL: http://codereview.chromium.org/506037

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

diff --git a/src/code-stubs.h b/src/code-stubs.h
index 9012fe7..b19f304 100644 (file)
--- a/src/code-stubs.h
+++ b/src/code-stubs.h
@@ -43,6 +43,7 @@ namespace internal {
   V(ConvertToDouble)                     \
   V(WriteInt32ToHeapNumber)              \
   V(StackCheck)                          \
+  V(FastNewClosure)                      \
   V(UnarySub)                            \
   V(RevertToNumber)                      \
   V(ToBoolean)                           \

diff --git a/src/codegen.h b/src/codegen.h
index 85a08d5..10bf5db 100644 (file)
--- a/src/codegen.h
+++ b/src/codegen.h
@@ -233,6 +233,17 @@ class StackCheckStub : public CodeStub {
 };
 
 
+class FastNewClosureStub : public CodeStub {
+ public:
+  void Generate(MacroAssembler* masm);
+
+ private:
+  const char* GetName() { return "FastNewClosureStub"; }
+  Major MajorKey() { return FastNewClosure; }
+  int MinorKey() { return 0; }
+};
+
+
 class InstanceofStub: public CodeStub {
  public:
   InstanceofStub() { }

diff --git a/src/factory.cc b/src/factory.cc
index 83775ef..2a80953 100644 (file)
--- a/src/factory.cc
+++ b/src/factory.cc
@@ -284,7 +284,8 @@ Handle<FixedArray> Factory::CopyFixedArray(Handle<FixedArray> array) {
 
 Handle<JSFunction> Factory::BaseNewFunctionFromBoilerplate(
     Handle<JSFunction> boilerplate,
-    Handle<Map> function_map) {
+    Handle<Map> function_map,
+    PretenureFlag pretenure) {
   ASSERT(boilerplate->IsBoilerplate());
   ASSERT(!boilerplate->has_initial_map());
   ASSERT(!boilerplate->has_prototype());
@@ -292,20 +293,22 @@ Handle<JSFunction> Factory::BaseNewFunctionFromBoilerplate(
   ASSERT(boilerplate->elements() == Heap::empty_fixed_array());
   CALL_HEAP_FUNCTION(Heap::AllocateFunction(*function_map,
                                             boilerplate->shared(),
-                                            Heap::the_hole_value()),
+                                            Heap::the_hole_value(),
+                                            pretenure),
                      JSFunction);
 }
 
 
 Handle<JSFunction> Factory::NewFunctionFromBoilerplate(
     Handle<JSFunction> boilerplate,
-    Handle<Context> context) {
-  Handle<JSFunction> result =
-      BaseNewFunctionFromBoilerplate(boilerplate, Top::function_map());
+    Handle<Context> context,
+    PretenureFlag pretenure) {
+  Handle<JSFunction> result = BaseNewFunctionFromBoilerplate(
+      boilerplate, Top::function_map(), pretenure);
   result->set_context(*context);
   int number_of_literals = boilerplate->NumberOfLiterals();
   Handle<FixedArray> literals =
-      Factory::NewFixedArray(number_of_literals, TENURED);
+      Factory::NewFixedArray(number_of_literals, pretenure);
   if (number_of_literals > 0) {
     // Store the object, regexp and array functions in the literals
     // array prefix.  These functions will be used when creating

diff --git a/src/factory.h b/src/factory.h
index 951c043..fd277f2 100644 (file)
--- a/src/factory.h
+++ b/src/factory.h
@@ -219,7 +219,8 @@ class Factory : public AllStatic {
 
   static Handle<JSFunction> NewFunctionFromBoilerplate(
       Handle<JSFunction> boilerplate,
-      Handle<Context> context);
+      Handle<Context> context,
+      PretenureFlag pretenure = TENURED);
 
   static Handle<Code> NewCode(const CodeDesc& desc,
                               ZoneScopeInfo* sinfo,
@@ -374,7 +375,8 @@ class Factory : public AllStatic {
 
   static Handle<JSFunction> BaseNewFunctionFromBoilerplate(
       Handle<JSFunction> boilerplate,
-      Handle<Map> function_map);
+      Handle<Map> function_map,
+      PretenureFlag pretenure);
 
   // Create a new map cache.
   static Handle<MapCache> NewMapCache(int at_least_space_for);

diff --git a/src/heap.cc b/src/heap.cc
index b9aa95c..3de5ba8 100644 (file)
--- a/src/heap.cc
+++ b/src/heap.cc
@@ -2233,8 +2233,11 @@ Object* Heap::AllocateFunctionPrototype(JSFunction* function) {
 
 Object* Heap::AllocateFunction(Map* function_map,
                                SharedFunctionInfo* shared,
-                               Object* prototype) {
-  Object* result = Allocate(function_map, OLD_POINTER_SPACE);
+                               Object* prototype,
+                               PretenureFlag pretenure) {
+  AllocationSpace space =
+      (pretenure == TENURED) ? OLD_POINTER_SPACE : NEW_SPACE;
+  Object* result = Allocate(function_map, space);
   if (result->IsFailure()) return result;
   return InitializeFunction(JSFunction::cast(result), shared, prototype);
 }

diff --git a/src/heap.h b/src/heap.h
index 05ff16c..12835e6 100644 (file)
--- a/src/heap.h
+++ b/src/heap.h
@@ -487,7 +487,8 @@ class Heap : public AllStatic {
   // Please note this does not perform a garbage collection.
   static Object* AllocateFunction(Map* function_map,
                                   SharedFunctionInfo* shared,
-                                  Object* prototype);
+                                  Object* prototype,
+                                  PretenureFlag pretenure = TENURED);
 
   // Indicies for direct access into argument objects.
   static const int arguments_callee_index = 0;

diff --git a/src/ia32/codegen-ia32.cc b/src/ia32/codegen-ia32.cc
index 417eb0f..b265588 100644 (file)
--- a/src/ia32/codegen-ia32.cc
+++ b/src/ia32/codegen-ia32.cc
@@ -3593,18 +3593,28 @@ void CodeGenerator::VisitDebuggerStatement(DebuggerStatement* node) {
 
 
 void CodeGenerator::InstantiateBoilerplate(Handle<JSFunction> boilerplate) {
-  // Call the runtime to instantiate the function boilerplate object.
-  // The inevitable call will sync frame elements to memory anyway, so
-  // we do it eagerly to allow us to push the arguments directly into
-  // place.
   ASSERT(boilerplate->IsBoilerplate());
-  frame_->SyncRange(0, frame_->element_count() - 1);
 
-  // Create a new closure.
-  frame_->EmitPush(esi);
-  frame_->EmitPush(Immediate(boilerplate));
-  Result result = frame_->CallRuntime(Runtime::kNewClosure, 2);
-  frame_->Push(&result);
+  // Use the fast case closure allocation code that allocated in new
+  // space for nested functions that don't need literals cloning.
+  if (scope()->is_function_scope() && boilerplate->NumberOfLiterals() == 0) {
+    FastNewClosureStub stub;
+    frame_->Push(boilerplate);
+    Result answer = frame_->CallStub(&stub, 1);
+    frame_->Push(&answer);
+  } else {
+    // Call the runtime to instantiate the function boilerplate
+    // object.  The inevitable call will sync frame elements to memory
+    // anyway, so we do it eagerly to allow us to push the arguments
+    // directly into place.
+    frame_->SyncRange(0, frame_->element_count() - 1);
+
+    // Create a new closure.
+    frame_->EmitPush(esi);
+    frame_->EmitPush(Immediate(boilerplate));
+    Result result = frame_->CallRuntime(Runtime::kNewClosure, 2);
+    frame_->Push(&result);
+  }
 }
 
 
@@ -6537,6 +6547,49 @@ void Reference::SetValue(InitState init_state) {
 }
 
 
+void FastNewClosureStub::Generate(MacroAssembler* masm) {
+  // Clone the boilerplate in new space. Set the context to the
+  // current context in esi.
+  Label gc;
+  __ AllocateInNewSpace(JSFunction::kSize, eax, ebx, ecx, &gc, TAG_OBJECT);
+
+  // Get the boilerplate function from the stack.
+  __ mov(edx, Operand(esp, 1 * kPointerSize));
+
+  // Compute the function map in the current global context and set that
+  // as the map of the allocated object.
+  __ mov(ecx, Operand(esi, Context::SlotOffset(Context::GLOBAL_INDEX)));
+  __ mov(ecx, FieldOperand(ecx, GlobalObject::kGlobalContextOffset));
+  __ mov(ecx, Operand(ecx, Context::SlotOffset(Context::FUNCTION_MAP_INDEX)));
+  __ mov(FieldOperand(eax, JSObject::kMapOffset), ecx);
+
+  // Clone the rest of the boilerplate fields. We don't have to update
+  // the write barrier because the allocated object is in new space.
+  for (int offset = kPointerSize;
+       offset < JSFunction::kSize;
+       offset += kPointerSize) {
+    if (offset == JSFunction::kContextOffset) {
+      __ mov(FieldOperand(eax, offset), esi);
+    } else {
+      __ mov(ebx, FieldOperand(edx, offset));
+      __ mov(FieldOperand(eax, offset), ebx);
+    }
+  }
+
+  // Return and remove the on-stack parameter.
+  __ ret(1 * kPointerSize);
+
+  // Create a new closure through the slower runtime call.
+  __ bind(&gc);
+  __ pop(ecx);  // Temporarily remove return address.
+  __ pop(edx);
+  __ push(esi);
+  __ push(edx);
+  __ push(ecx);  // Restore return address.
+  __ TailCallRuntime(ExternalReference(Runtime::kNewClosure), 2, 1);
+}
+
+
 // NOTE: The stub does not handle the inlined cases (Smis, Booleans, undefined).
 void ToBooleanStub::Generate(MacroAssembler* masm) {
   Label false_result, true_result, not_string;

diff --git a/src/ia32/stub-cache-ia32.cc b/src/ia32/stub-cache-ia32.cc
index 846b667..8aca1ef 100644 (file)
--- a/src/ia32/stub-cache-ia32.cc
+++ b/src/ia32/stub-cache-ia32.cc
@@ -1154,8 +1154,25 @@ Object* CallStubCompiler::CompileCallGlobal(JSObject* object,
   __ mov(edi, FieldOperand(edi, JSGlobalPropertyCell::kValueOffset));
 
   // Check that the cell contains the same function.
-  __ cmp(Operand(edi), Immediate(Handle<JSFunction>(function)));
-  __ j(not_equal, &miss, not_taken);
+  if (Heap::InNewSpace(function)) {
+    // We can't embed a pointer to a function in new space so we have
+    // to verify that the shared function info is unchanged. This has
+    // the nice side effect that multiple closures based on the same
+    // function can all use this call IC. Before we load through the
+    // function, we have to verify that it still is a function.
+    __ test(edi, Immediate(kSmiTagMask));
+    __ j(zero, &miss, not_taken);
+    __ CmpObjectType(edi, JS_FUNCTION_TYPE, ebx);
+    __ j(not_equal, &miss, not_taken);
+
+    // Check the shared function info. Make sure it hasn't changed.
+    __ cmp(FieldOperand(edi, JSFunction::kSharedFunctionInfoOffset),
+           Immediate(Handle<SharedFunctionInfo>(function->shared())));
+    __ j(not_equal, &miss, not_taken);
+  } else {
+    __ cmp(Operand(edi), Immediate(Handle<JSFunction>(function)));
+    __ j(not_equal, &miss, not_taken);
+  }
 
   // Patch the receiver on the stack with the global proxy.
   if (object->IsGlobalObject()) {

diff --git a/src/objects-inl.h b/src/objects-inl.h
index 8514a41..ce32eed 100644 (file)
--- a/src/objects-inl.h
+++ b/src/objects-inl.h
@@ -1499,7 +1499,7 @@ void DescriptorArray::Set(int descriptor_number, Descriptor* desc) {
   // Range check.
   ASSERT(descriptor_number < number_of_descriptors());
 
-  // Make sure non of the elements in desc are in new space.
+  // Make sure none of the elements in desc are in new space.
   ASSERT(!Heap::InNewSpace(desc->GetKey()));
   ASSERT(!Heap::InNewSpace(desc->GetValue()));
 

diff --git a/src/objects.cc b/src/objects.cc
index 0f8dca3..52f1f9a 100644 (file)
--- a/src/objects.cc
+++ b/src/objects.cc
@@ -1351,6 +1351,8 @@ Object* JSObject::AddFastProperty(String* name,
 Object* JSObject::AddConstantFunctionProperty(String* name,
                                               JSFunction* function,
                                               PropertyAttributes attributes) {
+  ASSERT(!Heap::InNewSpace(function));
+
   // Allocate new instance descriptors with (name, function) added
   ConstantFunctionDescriptor d(name, function, attributes);
   Object* new_descriptors =
@@ -1437,7 +1439,7 @@ Object* JSObject::AddProperty(String* name,
     // Ensure the descriptor array does not get too big.
     if (map()->instance_descriptors()->number_of_descriptors() <
         DescriptorArray::kMaxNumberOfDescriptors) {
-      if (value->IsJSFunction()) {
+      if (value->IsJSFunction() && !Heap::InNewSpace(value)) {
         return AddConstantFunctionProperty(name,
                                            JSFunction::cast(value),
                                            attributes);
@@ -3254,7 +3256,8 @@ Object* DescriptorArray::Allocate(int number_of_descriptors) {
     return Heap::empty_descriptor_array();
   }
   // Allocate the array of keys.
-  Object* array = Heap::AllocateFixedArray(ToKeyIndex(number_of_descriptors));
+  Object* array =
+      Heap::AllocateFixedArray(ToKeyIndex(number_of_descriptors));
   if (array->IsFailure()) return array;
   // Do not use DescriptorArray::cast on incomplete object.
   FixedArray* result = FixedArray::cast(array);
@@ -7962,7 +7965,10 @@ Object* StringDictionary::TransformPropertiesToFastFor(
       PropertyType type = DetailsAt(i).type();
       ASSERT(type != FIELD);
       instance_descriptor_length++;
-      if (type == NORMAL && !value->IsJSFunction()) number_of_fields += 1;
+      if (type == NORMAL &&
+          (!value->IsJSFunction() || Heap::InNewSpace(value))) {
+        number_of_fields += 1;
+      }
     }
   }
 
@@ -7993,7 +7999,7 @@ Object* StringDictionary::TransformPropertiesToFastFor(
       PropertyDetails details = DetailsAt(i);
       PropertyType type = details.type();
 
-      if (value->IsJSFunction()) {
+      if (value->IsJSFunction() && !Heap::InNewSpace(value)) {
         ConstantFunctionDescriptor d(String::cast(key),
                                      JSFunction::cast(value),
                                      details.attributes(),

diff --git a/src/objects.h b/src/objects.h
index 671978a..1545bbf 100644 (file)
--- a/src/objects.h
+++ b/src/objects.h
@@ -1662,6 +1662,7 @@ class DescriptorArray: public FixedArray {
  public:
   // Is this the singleton empty_descriptor_array?
   inline bool IsEmpty();
+
   // Returns the number of descriptors in the array.
   int number_of_descriptors() {
     return IsEmpty() ? 0 : length() - kFirstIndex;
@@ -1801,13 +1802,15 @@ class DescriptorArray: public FixedArray {
   static int ToKeyIndex(int descriptor_number) {
     return descriptor_number+kFirstIndex;
   }
-  static int ToValueIndex(int descriptor_number) {
-    return descriptor_number << 1;
-  }
+
   static int ToDetailsIndex(int descriptor_number) {
     return( descriptor_number << 1) + 1;
   }
 
+  static int ToValueIndex(int descriptor_number) {
+    return descriptor_number << 1;
+  }
+
   bool is_null_descriptor(int descriptor_number) {
     return PropertyDetails(GetDetails(descriptor_number)).type() ==
         NULL_DESCRIPTOR;

diff --git a/src/runtime.cc b/src/runtime.cc
index ac61de2..8349833 100644 (file)
--- a/src/runtime.cc
+++ b/src/runtime.cc
@@ -720,7 +720,7 @@ static Object* Runtime_DeclareGlobals(Arguments args) {
       // Copy the function and update its context. Use it as value.
       Handle<JSFunction> boilerplate = Handle<JSFunction>::cast(value);
       Handle<JSFunction> function =
-          Factory::NewFunctionFromBoilerplate(boilerplate, context);
+          Factory::NewFunctionFromBoilerplate(boilerplate, context, TENURED);
       value = function;
     }
 
@@ -4502,8 +4502,11 @@ static Object* Runtime_NewClosure(Arguments args) {
   CONVERT_ARG_CHECKED(Context, context, 0);
   CONVERT_ARG_CHECKED(JSFunction, boilerplate, 1);
 
+  PretenureFlag pretenure = (context->global_context() == *context)
+      ? TENURED       // Allocate global closures in old space.
+      : NOT_TENURED;  // Allocate local closures in new space.
   Handle<JSFunction> result =
-      Factory::NewFunctionFromBoilerplate(boilerplate, context);
+      Factory::NewFunctionFromBoilerplate(boilerplate, context, pretenure);
   return *result;
 }
 
@@ -5219,7 +5222,7 @@ static Object* Runtime_CompileString(Arguments args) {
                                                          validate);
   if (boilerplate.is_null()) return Failure::Exception();
   Handle<JSFunction> fun =
-      Factory::NewFunctionFromBoilerplate(boilerplate, context);
+      Factory::NewFunctionFromBoilerplate(boilerplate, context, NOT_TENURED);
   return *fun;
 }
 
@@ -5247,7 +5250,7 @@ static Object* CompileDirectEval(Handle<String> source) {
       Compiler::DONT_VALIDATE_JSON);
   if (boilerplate.is_null()) return Failure::Exception();
   Handle<JSFunction> fun =
-    Factory::NewFunctionFromBoilerplate(boilerplate, context);
+      Factory::NewFunctionFromBoilerplate(boilerplate, context, NOT_TENURED);
   return *fun;
 }