From 1e8413e1883bd33782eb5916561bbcb7e5c31442 Mon Sep 17 00:00:00 2001 From: "ager@chromium.org" Date: Mon, 22 Nov 2010 09:57:21 +0000 Subject: [PATCH] Force pretenuring of closures that are immediately assigned to properties. For these closures we would like to be able to use constant functions and for that we need the closures allocated in old space. Review URL: http://codereview.chromium.org/5220007 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@5866 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/arm/code-stubs-arm.cc | 5 +++-- src/arm/codegen-arm.cc | 16 +++++++++++----- src/arm/codegen-arm.h | 3 ++- src/arm/full-codegen-arm.cc | 13 +++++++++---- src/ast.h | 7 ++++++- src/full-codegen.cc | 4 ++-- src/full-codegen.h | 2 +- src/ia32/code-stubs-ia32.cc | 3 ++- src/ia32/codegen-ia32.cc | 16 +++++++++++----- src/ia32/codegen-ia32.h | 3 ++- src/ia32/full-codegen-ia32.cc | 12 +++++++++--- src/parser.cc | 6 ++++++ src/runtime.cc | 15 ++++++++++----- src/runtime.h | 2 +- src/x64/code-stubs-x64.cc | 3 ++- src/x64/codegen-x64.cc | 16 +++++++++++----- src/x64/codegen-x64.h | 3 ++- src/x64/full-codegen-x64.cc | 10 +++++++--- 18 files changed, 97 insertions(+), 42 deletions(-) diff --git a/src/arm/code-stubs-arm.cc b/src/arm/code-stubs-arm.cc index b3b0766a8f..76a610b7be 100644 --- a/src/arm/code-stubs-arm.cc +++ b/src/arm/code-stubs-arm.cc @@ -100,8 +100,9 @@ void FastNewClosureStub::Generate(MacroAssembler* masm) { // Create a new closure through the slower runtime call. __ bind(&gc); - __ Push(cp, r3); - __ TailCallRuntime(Runtime::kNewClosure, 2, 1); + __ LoadRoot(r4, Heap::kFalseValueRootIndex); + __ Push(cp, r3, r4); + __ TailCallRuntime(Runtime::kNewClosure, 3, 1); } diff --git a/src/arm/codegen-arm.cc b/src/arm/codegen-arm.cc index fe7ea8bd80..3e6743afaf 100644 --- a/src/arm/codegen-arm.cc +++ b/src/arm/codegen-arm.cc @@ -3103,10 +3103,13 @@ void CodeGenerator::VisitDebuggerStatement(DebuggerStatement* node) { void CodeGenerator::InstantiateFunction( - Handle function_info) { + Handle function_info, + bool pretenure) { // Use the fast case closure allocation code that allocates in new // space for nested functions that don't need literals cloning. - if (scope()->is_function_scope() && function_info->num_literals() == 0) { + if (scope()->is_function_scope() && + function_info->num_literals() == 0 && + !pretenure) { FastNewClosureStub stub; frame_->EmitPush(Operand(function_info)); frame_->SpillAll(); @@ -3116,7 +3119,10 @@ void CodeGenerator::InstantiateFunction( // Create a new closure. frame_->EmitPush(cp); frame_->EmitPush(Operand(function_info)); - frame_->CallRuntime(Runtime::kNewClosure, 2); + frame_->EmitPush(Operand(pretenure + ? Factory::true_value() + : Factory::false_value())); + frame_->CallRuntime(Runtime::kNewClosure, 3); frame_->EmitPush(r0); } } @@ -3136,7 +3142,7 @@ void CodeGenerator::VisitFunctionLiteral(FunctionLiteral* node) { ASSERT(frame_->height() == original_height); return; } - InstantiateFunction(function_info); + InstantiateFunction(function_info, node->pretenure()); ASSERT_EQ(original_height + 1, frame_->height()); } @@ -3147,7 +3153,7 @@ void CodeGenerator::VisitSharedFunctionInfoLiteral( int original_height = frame_->height(); #endif Comment cmnt(masm_, "[ SharedFunctionInfoLiteral"); - InstantiateFunction(node->shared_function_info()); + InstantiateFunction(node->shared_function_info(), false); ASSERT_EQ(original_height + 1, frame_->height()); } diff --git a/src/arm/codegen-arm.h b/src/arm/codegen-arm.h index 9495f1b608..6905d2331e 100644 --- a/src/arm/codegen-arm.h +++ b/src/arm/codegen-arm.h @@ -449,7 +449,8 @@ class CodeGenerator: public AstVisitor { void DeclareGlobals(Handle pairs); // Instantiate the function based on the shared function info. - void InstantiateFunction(Handle function_info); + void InstantiateFunction(Handle function_info, + bool pretenure); // Support for type checks. void GenerateIsSmi(ZoneList* args); diff --git a/src/arm/full-codegen-arm.cc b/src/arm/full-codegen-arm.cc index f0fa5a7749..f04015bd78 100644 --- a/src/arm/full-codegen-arm.cc +++ b/src/arm/full-codegen-arm.cc @@ -860,18 +860,23 @@ void FullCodeGenerator::VisitForInStatement(ForInStatement* stmt) { } -void FullCodeGenerator::EmitNewClosure(Handle info) { +void FullCodeGenerator::EmitNewClosure(Handle info, + bool pretenure) { // Use the fast case closure allocation code that allocates in new // space for nested functions that don't need literals cloning. - if (scope()->is_function_scope() && info->num_literals() == 0) { + if (scope()->is_function_scope() && + info->num_literals() == 0 && + !pretenure) { FastNewClosureStub stub; __ mov(r0, Operand(info)); __ push(r0); __ CallStub(&stub); } else { __ mov(r0, Operand(info)); - __ Push(cp, r0); - __ CallRuntime(Runtime::kNewClosure, 2); + __ LoadRoot(r1, pretenure ? Heap::kTrueValueRootIndex + : Heap::kFalseValueRootIndex); + __ Push(cp, r0, r1); + __ CallRuntime(Runtime::kNewClosure, 3); } context()->Plug(r0); } diff --git a/src/ast.h b/src/ast.h index 04c2977570..0846dbc537 100644 --- a/src/ast.h +++ b/src/ast.h @@ -1416,7 +1416,8 @@ class FunctionLiteral: public Expression { contains_loops_(contains_loops), function_token_position_(RelocInfo::kNoPosition), inferred_name_(Heap::empty_string()), - try_full_codegen_(false) { + try_full_codegen_(false), + pretenure_(false) { #ifdef DEBUG already_compiled_ = false; #endif @@ -1459,6 +1460,9 @@ class FunctionLiteral: public Expression { bool try_full_codegen() { return try_full_codegen_; } void set_try_full_codegen(bool flag) { try_full_codegen_ = flag; } + bool pretenure() { return pretenure_; } + void set_pretenure(bool value) { pretenure_ = value; } + #ifdef DEBUG void mark_as_compiled() { ASSERT(!already_compiled_); @@ -1482,6 +1486,7 @@ class FunctionLiteral: public Expression { int function_token_position_; Handle inferred_name_; bool try_full_codegen_; + bool pretenure_; #ifdef DEBUG bool already_compiled_; #endif diff --git a/src/full-codegen.cc b/src/full-codegen.cc index 19971a2c48..a890f15974 100644 --- a/src/full-codegen.cc +++ b/src/full-codegen.cc @@ -1168,14 +1168,14 @@ void FullCodeGenerator::VisitFunctionLiteral(FunctionLiteral* expr) { SetStackOverflow(); return; } - EmitNewClosure(function_info); + EmitNewClosure(function_info, expr->pretenure()); } void FullCodeGenerator::VisitSharedFunctionInfoLiteral( SharedFunctionInfoLiteral* expr) { Comment cmnt(masm_, "[ SharedFunctionInfoLiteral"); - EmitNewClosure(expr->shared_function_info()); + EmitNewClosure(expr->shared_function_info(), false); } diff --git a/src/full-codegen.h b/src/full-codegen.h index 6a1def6ee1..97a56bd908 100644 --- a/src/full-codegen.h +++ b/src/full-codegen.h @@ -348,7 +348,7 @@ class FullCodeGenerator: public AstVisitor { // Platform-specific support for allocating a new closure based on // the given function info. - void EmitNewClosure(Handle info); + void EmitNewClosure(Handle info, bool pretenure); // Platform-specific support for compiling assignments. diff --git a/src/ia32/code-stubs-ia32.cc b/src/ia32/code-stubs-ia32.cc index 6ac89bfbac..5975ad27db 100644 --- a/src/ia32/code-stubs-ia32.cc +++ b/src/ia32/code-stubs-ia32.cc @@ -80,8 +80,9 @@ void FastNewClosureStub::Generate(MacroAssembler* masm) { __ pop(edx); __ push(esi); __ push(edx); + __ push(Immediate(Factory::false_value())); __ push(ecx); // Restore return address. - __ TailCallRuntime(Runtime::kNewClosure, 2, 1); + __ TailCallRuntime(Runtime::kNewClosure, 3, 1); } diff --git a/src/ia32/codegen-ia32.cc b/src/ia32/codegen-ia32.cc index 060ccce845..f5ab357ff1 100644 --- a/src/ia32/codegen-ia32.cc +++ b/src/ia32/codegen-ia32.cc @@ -4897,7 +4897,8 @@ void CodeGenerator::VisitDebuggerStatement(DebuggerStatement* node) { Result CodeGenerator::InstantiateFunction( - Handle function_info) { + Handle function_info, + bool pretenure) { // 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. @@ -4905,7 +4906,9 @@ Result CodeGenerator::InstantiateFunction( // Use the fast case closure allocation code that allocates in new // space for nested functions that don't need literals cloning. - if (scope()->is_function_scope() && function_info->num_literals() == 0) { + if (scope()->is_function_scope() && + function_info->num_literals() == 0 && + !pretenure) { FastNewClosureStub stub; frame()->EmitPush(Immediate(function_info)); return frame()->CallStub(&stub, 1); @@ -4914,7 +4917,10 @@ Result CodeGenerator::InstantiateFunction( // shared function info. frame()->EmitPush(esi); frame()->EmitPush(Immediate(function_info)); - return frame()->CallRuntime(Runtime::kNewClosure, 2); + frame()->EmitPush(Immediate(pretenure + ? Factory::true_value() + : Factory::false_value())); + return frame()->CallRuntime(Runtime::kNewClosure, 3); } } @@ -4930,7 +4936,7 @@ void CodeGenerator::VisitFunctionLiteral(FunctionLiteral* node) { SetStackOverflow(); return; } - Result result = InstantiateFunction(function_info); + Result result = InstantiateFunction(function_info, node->pretenure()); frame()->Push(&result); } @@ -4939,7 +4945,7 @@ void CodeGenerator::VisitSharedFunctionInfoLiteral( SharedFunctionInfoLiteral* node) { ASSERT(!in_safe_int32_mode()); Comment cmnt(masm_, "[ SharedFunctionInfoLiteral"); - Result result = InstantiateFunction(node->shared_function_info()); + Result result = InstantiateFunction(node->shared_function_info(), false); frame()->Push(&result); } diff --git a/src/ia32/codegen-ia32.h b/src/ia32/codegen-ia32.h index f15e08c540..d1a2036cb7 100644 --- a/src/ia32/codegen-ia32.h +++ b/src/ia32/codegen-ia32.h @@ -625,7 +625,8 @@ class CodeGenerator: public AstVisitor { void DeclareGlobals(Handle pairs); // Instantiate the function based on the shared function info. - Result InstantiateFunction(Handle function_info); + Result InstantiateFunction(Handle function_info, + bool pretenure); // Support for types. void GenerateIsSmi(ZoneList* args); diff --git a/src/ia32/full-codegen-ia32.cc b/src/ia32/full-codegen-ia32.cc index b742d352c4..3adc48a739 100644 --- a/src/ia32/full-codegen-ia32.cc +++ b/src/ia32/full-codegen-ia32.cc @@ -880,17 +880,23 @@ void FullCodeGenerator::VisitForInStatement(ForInStatement* stmt) { } -void FullCodeGenerator::EmitNewClosure(Handle info) { +void FullCodeGenerator::EmitNewClosure(Handle info, + bool pretenure) { // Use the fast case closure allocation code that allocates in new // space for nested functions that don't need literals cloning. - if (scope()->is_function_scope() && info->num_literals() == 0) { + if (scope()->is_function_scope() && + info->num_literals() == 0 && + !pretenure) { FastNewClosureStub stub; __ push(Immediate(info)); __ CallStub(&stub); } else { __ push(esi); __ push(Immediate(info)); - __ CallRuntime(Runtime::kNewClosure, 2); + __ push(Immediate(pretenure + ? Factory::true_value() + : Factory::false_value())); + __ CallRuntime(Runtime::kNewClosure, 3); } context()->Plug(eax); } diff --git a/src/parser.cc b/src/parser.cc index 8b5e8270b3..7e4a51e2fa 100644 --- a/src/parser.cc +++ b/src/parser.cc @@ -2276,6 +2276,12 @@ Expression* Parser::ParseAssignmentExpression(bool accept_IN, bool* ok) { temp_scope_->AddProperty(); } + // If we assign a function literal to a property we pretenure the + // literal so it can be added as a constant function property. + if (property != NULL && right->AsFunctionLiteral() != NULL) { + right->AsFunctionLiteral()->set_pretenure(true); + } + if (fni_ != NULL) { // Check if the right hand side is a call to avoid inferring a // name if we're dealing with "a = function(){...}();"-like diff --git a/src/runtime.cc b/src/runtime.cc index 3271148980..08f1865e71 100644 --- a/src/runtime.cc +++ b/src/runtime.cc @@ -6341,15 +6341,20 @@ static MaybeObject* Runtime_NewArgumentsFast(Arguments args) { static MaybeObject* Runtime_NewClosure(Arguments args) { HandleScope scope; - ASSERT(args.length() == 2); + ASSERT(args.length() == 3); CONVERT_ARG_CHECKED(Context, context, 0); CONVERT_ARG_CHECKED(SharedFunctionInfo, shared, 1); + CONVERT_BOOLEAN_CHECKED(pretenure, args[2]); - PretenureFlag pretenure = (context->global_context() == *context) - ? TENURED // Allocate global closures in old space. - : NOT_TENURED; // Allocate local closures in new space. + // Allocate global closures in old space and allocate local closures + // in new space. Additionally pretenure closures that are assigned + // directly to properties. + pretenure = pretenure || (context->global_context() == *context); + PretenureFlag pretenure_flag = pretenure ? TENURED : NOT_TENURED; Handle result = - Factory::NewFunctionFromSharedFunctionInfo(shared, context, pretenure); + Factory::NewFunctionFromSharedFunctionInfo(shared, + context, + pretenure_flag); return *result; } diff --git a/src/runtime.h b/src/runtime.h index 3436f124ba..f9ebbc42ef 100644 --- a/src/runtime.h +++ b/src/runtime.h @@ -262,7 +262,7 @@ namespace internal { F(CreateCatchExtensionObject, 2, 1) \ \ /* Statements */ \ - F(NewClosure, 2, 1) \ + F(NewClosure, 3, 1) \ F(NewObject, 1, 1) \ F(NewObjectFromBound, 2, 1) \ F(FinalizeInstanceSize, 1, 1) \ diff --git a/src/x64/code-stubs-x64.cc b/src/x64/code-stubs-x64.cc index 04173e1afb..14e352731f 100644 --- a/src/x64/code-stubs-x64.cc +++ b/src/x64/code-stubs-x64.cc @@ -80,8 +80,9 @@ void FastNewClosureStub::Generate(MacroAssembler* masm) { __ pop(rdx); __ push(rsi); __ push(rdx); + __ Push(Factory::false_value()); __ push(rcx); // Restore return address. - __ TailCallRuntime(Runtime::kNewClosure, 2, 1); + __ TailCallRuntime(Runtime::kNewClosure, 3, 1); } diff --git a/src/x64/codegen-x64.cc b/src/x64/codegen-x64.cc index 395f7c39ca..5abf3c838c 100644 --- a/src/x64/codegen-x64.cc +++ b/src/x64/codegen-x64.cc @@ -4244,7 +4244,8 @@ void CodeGenerator::VisitDebuggerStatement(DebuggerStatement* node) { void CodeGenerator::InstantiateFunction( - Handle function_info) { + Handle function_info, + bool pretenure) { // 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. @@ -4252,7 +4253,9 @@ void CodeGenerator::InstantiateFunction( // Use the fast case closure allocation code that allocates in new // space for nested functions that don't need literals cloning. - if (scope()->is_function_scope() && function_info->num_literals() == 0) { + if (scope()->is_function_scope() && + function_info->num_literals() == 0 && + !pretenure) { FastNewClosureStub stub; frame_->Push(function_info); Result answer = frame_->CallStub(&stub, 1); @@ -4262,7 +4265,10 @@ void CodeGenerator::InstantiateFunction( // shared function info. frame_->EmitPush(rsi); frame_->EmitPush(function_info); - Result result = frame_->CallRuntime(Runtime::kNewClosure, 2); + frame_->EmitPush(pretenure + ? Factory::true_value() + : Factory::false_value()); + Result result = frame_->CallRuntime(Runtime::kNewClosure, 3); frame_->Push(&result); } } @@ -4279,14 +4285,14 @@ void CodeGenerator::VisitFunctionLiteral(FunctionLiteral* node) { SetStackOverflow(); return; } - InstantiateFunction(function_info); + InstantiateFunction(function_info, node->pretenure()); } void CodeGenerator::VisitSharedFunctionInfoLiteral( SharedFunctionInfoLiteral* node) { Comment cmnt(masm_, "[ SharedFunctionInfoLiteral"); - InstantiateFunction(node->shared_function_info()); + InstantiateFunction(node->shared_function_info(), false); } diff --git a/src/x64/codegen-x64.h b/src/x64/codegen-x64.h index 2bc82a46f5..1a5e7df31c 100644 --- a/src/x64/codegen-x64.h +++ b/src/x64/codegen-x64.h @@ -585,7 +585,8 @@ class CodeGenerator: public AstVisitor { void DeclareGlobals(Handle pairs); // Instantiate the function based on the shared function info. - void InstantiateFunction(Handle function_info); + void InstantiateFunction(Handle function_info, + bool pretenure); // Support for type checks. void GenerateIsSmi(ZoneList* args); diff --git a/src/x64/full-codegen-x64.cc b/src/x64/full-codegen-x64.cc index 4dbc655d3f..ee80169bb9 100644 --- a/src/x64/full-codegen-x64.cc +++ b/src/x64/full-codegen-x64.cc @@ -837,17 +837,21 @@ void FullCodeGenerator::VisitForInStatement(ForInStatement* stmt) { } -void FullCodeGenerator::EmitNewClosure(Handle info) { +void FullCodeGenerator::EmitNewClosure(Handle info, + bool pretenure) { // Use the fast case closure allocation code that allocates in new // space for nested functions that don't need literals cloning. - if (scope()->is_function_scope() && info->num_literals() == 0) { + if (scope()->is_function_scope() && + info->num_literals() == 0 && + !pretenure) { FastNewClosureStub stub; __ Push(info); __ CallStub(&stub); } else { __ push(rsi); __ Push(info); - __ CallRuntime(Runtime::kNewClosure, 2); + __ Push(pretenure ? Factory::true_value() : Factory::false_value()); + __ CallRuntime(Runtime::kNewClosure, 3); } context()->Plug(rax); }