Allocate arguments object on-demand instead of at function entry.

This allows Function.prototype.apply to not allocate the objects 
and copy the arguments directly from the stack.
Review URL: http://codereview.chromium.org/147075

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

src/accessors.cc

@@ -511,7 +511,10 @@ Object* Accessors::FunctionGetArguments(Object* object, void*) {
     // If there is an arguments variable in the stack, we return that.
     int index = ScopeInfo<>::StackSlotIndex(frame->code(),
                                             Heap::arguments_symbol());
-    if (index >= 0) return frame->GetExpression(index);
+    if (index >= 0) {
+      Handle<Object> arguments = Handle<Object>(frame->GetExpression(index));
+      if (!arguments->IsTheHole()) return *arguments;
+    }
 
     // If there isn't an arguments variable in the stack, we need to
     // find the frame that holds the actual arguments passed to the

src/ast.cc

@@ -68,7 +68,7 @@ VariableProxy::VariableProxy(Handle<String> name,
   // names must be canonicalized for fast equality checks
   ASSERT(name->IsSymbol());
   // at least one access, otherwise no need for a VariableProxy
-  var_uses_.RecordAccess(1);
+  var_uses_.RecordRead(1);
 }
 
 

src/ast.h

@@ -802,13 +802,20 @@ class VariableProxy: public Expression {
   Variable* AsVariable() {
     return this == NULL || var_ == NULL ? NULL : var_->AsVariable();
   }
+
   virtual bool IsValidLeftHandSide() {
     return var_ == NULL ? true : var_->IsValidLeftHandSide();
   }
+
   bool IsVariable(Handle<String> n) {
     return !is_this() && name().is_identical_to(n);
   }
 
+  bool IsArguments() {
+    Variable* variable = AsVariable();
+    return (variable == NULL) ? false : variable->is_arguments();
+  }
+
   // If this assertion fails it means that some code has tried to
   // treat the special "this" variable as an ordinary variable with
   // the name "this".
@@ -890,12 +897,13 @@ class Slot: public Expression {
   virtual void Accept(AstVisitor* v);
 
   // Type testing & conversion
-  virtual Slot* AsSlot()  { return this; }
+  virtual Slot* AsSlot() { return this; }
 
   // Accessors
-  Variable* var() const  { return var_; }
-  Type type() const  { return type_; }
-  int index() const  { return index_; }
+  Variable* var() const { return var_; }
+  Type type() const { return type_; }
+  int index() const { return index_; }
+  bool is_arguments() const { return var_->is_arguments(); }
 
  private:
   Variable* var_;

src/ia32/codegen-ia32.h

@@ -273,6 +273,14 @@ class CodeGenState BASE_EMBEDDED {
 };
 
 
+// -------------------------------------------------------------------------
+// Arguments allocation mode
+
+enum ArgumentsAllocationMode {
+  NO_ARGUMENTS_ALLOCATION,
+  EAGER_ARGUMENTS_ALLOCATION,
+  LAZY_ARGUMENTS_ALLOCATION
+};
 
 
 // -------------------------------------------------------------------------
@@ -332,12 +340,11 @@ class CodeGenerator: public AstVisitor {
 
   // Accessors
   Scope* scope() const { return scope_; }
+  bool is_eval() { return is_eval_; }
 
   // Generating deferred code.
   void ProcessDeferred();
 
-  bool is_eval() { return is_eval_; }
-
   // State
   TypeofState typeof_state() const { return state_->typeof_state(); }
   ControlDestination* destination() const { return state_->destination(); }
@@ -373,6 +380,12 @@ class CodeGenerator: public AstVisitor {
   // target (which can not be done more than once).
   void GenerateReturnSequence(Result* return_value);
 
+  // Returns the arguments allocation mode.
+  ArgumentsAllocationMode ArgumentsMode() const;
+
+  // Store the arguments object and allocate it if necessary.
+  Result StoreArgumentsObject(bool initial);
+
   // The following are used by class Reference.
   void LoadReference(Reference* ref);
   void UnloadReference(Reference* ref);
@@ -408,6 +421,7 @@ class CodeGenerator: public AstVisitor {
 
   // Read a value from a slot and leave it on top of the expression stack.
   void LoadFromSlot(Slot* slot, TypeofState typeof_state);
+  void LoadFromSlotCheckForArguments(Slot* slot, TypeofState typeof_state);
   Result LoadFromGlobalSlotCheckExtensions(Slot* slot,
                                            TypeofState typeof_state,
                                            JumpTarget* slow);
@@ -470,6 +484,14 @@ class CodeGenerator: public AstVisitor {
 
   void CallWithArguments(ZoneList<Expression*>* arguments, int position);
 
+  // Use an optimized version of Function.prototype.apply that avoid
+  // allocating the arguments object and just copies the arguments
+  // from the stack.
+  void CallApplyLazy(Property* apply,
+                     Expression* receiver,
+                     VariableProxy* arguments,
+                     int position);
+
   void CheckStack();
 
   struct InlineRuntimeLUT {

src/parser.cc

@@ -1582,7 +1582,8 @@ VariableProxy* AstBuildingParser::Declare(Handle<String> name,
   // For global const variables we bind the proxy to a variable.
   if (mode == Variable::CONST && top_scope_->is_global_scope()) {
     ASSERT(resolve);  // should be set by all callers
-    var = NEW(Variable(top_scope_, name, Variable::CONST, true, false));
+    Variable::Kind kind = Variable::NORMAL;
+    var = NEW(Variable(top_scope_, name, Variable::CONST, true, kind));
   }
 
   // If requested and we have a local variable, bind the proxy to the variable

src/runtime.js

@@ -391,8 +391,9 @@ function CALL_NON_FUNCTION_AS_CONSTRUCTOR() {
 
 function APPLY_PREPARE(args) {
   var length;
-  // First check whether length is a positive Smi and args is an array.  This is the
-  // fast case.  If this fails, we do the slow case that takes care of more eventualities
+  // First check whether length is a positive Smi and args is an
+  // array. This is the fast case. If this fails, we do the slow case
+  // that takes care of more eventualities.
   if (%_IsArray(args)) {
     length = args.length;
     if (%_IsSmi(length) && length >= 0 && length < 0x800000 && IS_FUNCTION(this)) {

src/scopes.cc

@@ -81,12 +81,12 @@ Variable* LocalsMap::Declare(Scope* scope,
                              Handle<String> name,
                              Variable::Mode mode,
                              bool is_valid_LHS,
-                             bool is_this) {
+                             Variable::Kind kind) {
   HashMap::Entry* p = HashMap::Lookup(name.location(), name->Hash(), true);
   if (p->value == NULL) {
     // The variable has not been declared yet -> insert it.
     ASSERT(p->key == name.location());
-    p->value = new Variable(scope, name, mode, is_valid_LHS, is_this);
+    p->value = new Variable(scope, name, mode, is_valid_LHS, kind);
   }
   return reinterpret_cast<Variable*>(p->value);
 }
@@ -169,7 +169,8 @@ void Scope::Initialize(bool inside_with) {
   // such parameter is 'this' which is passed on the stack when
   // invoking scripts
   { Variable* var =
-      locals_.Declare(this, Factory::this_symbol(), Variable::VAR, false, true);
+        locals_.Declare(this, Factory::this_symbol(), Variable::VAR,
+                        false, Variable::THIS);
     var->rewrite_ = new Slot(var, Slot::PARAMETER, -1);
     receiver_ = new VariableProxy(Factory::this_symbol(), true, false);
     receiver_->BindTo(var);
@@ -179,7 +180,8 @@ void Scope::Initialize(bool inside_with) {
     // Declare 'arguments' variable which exists in all functions.
     // Note that it may never be accessed, in which case it won't
     // be allocated during variable allocation.
-    Declare(Factory::arguments_symbol(), Variable::VAR);
+    locals_.Declare(this, Factory::arguments_symbol(), Variable::VAR,
+                    true, Variable::ARGUMENTS);
   }
 }
 
@@ -203,7 +205,7 @@ Variable* Scope::Lookup(Handle<String> name) {
 
 Variable* Scope::DeclareFunctionVar(Handle<String> name) {
   ASSERT(is_function_scope() && function_ == NULL);
-  function_ = new Variable(this, name, Variable::CONST, true, false);
+  function_ = new Variable(this, name, Variable::CONST, true, Variable::NORMAL);
   return function_;
 }
 
@@ -213,7 +215,7 @@ Variable* Scope::Declare(Handle<String> name, Variable::Mode mode) {
   // INTERNAL variables are allocated explicitly, and TEMPORARY
   // variables are allocated via NewTemporary().
   ASSERT(mode == Variable::VAR || mode == Variable::CONST);
-  return locals_.Declare(this, name, mode, true, false);
+  return locals_.Declare(this, name, mode, true, Variable::NORMAL);
 }
 
 
@@ -247,7 +249,8 @@ void Scope::RemoveUnresolved(VariableProxy* var) {
 
 
 VariableProxy* Scope::NewTemporary(Handle<String> name) {
-  Variable* var = new Variable(this, name, Variable::TEMPORARY, true, false);
+  Variable* var = new Variable(this, name, Variable::TEMPORARY, true,
+                               Variable::NORMAL);
   VariableProxy* tmp = new VariableProxy(name, false, false);
   tmp->BindTo(var);
   temps_.Add(var);
@@ -503,7 +506,7 @@ Variable* Scope::NonLocal(Handle<String> name, Variable::Mode mode) {
   Variable* var = map->Lookup(name);
   if (var == NULL) {
     // Declare a new non-local.
-    var = map->Declare(NULL, name, mode, true, false);
+    var = map->Declare(NULL, name, mode, true, Variable::NORMAL);
     // Allocate it by giving it a dynamic lookup.
     var->rewrite_ = new Slot(var, Slot::LOOKUP, -1);
   }
@@ -619,7 +622,7 @@ void Scope::ResolveVariable(Scope* global_scope,
         // We must have a global variable.
         ASSERT(global_scope != NULL);
         var = new Variable(global_scope, proxy->name(),
-                           Variable::DYNAMIC, true, false);
+                           Variable::DYNAMIC, true, Variable::NORMAL);
 
       } else if (scope_inside_with_) {
         // If we are inside a with statement we give up and look up
@@ -797,7 +800,7 @@ void Scope::AllocateParameterLocals() {
     // are never allocated in the context).
     Variable* arguments_shadow =
         new Variable(this, Factory::arguments_shadow_symbol(),
-                     Variable::INTERNAL, true, false);
+                     Variable::INTERNAL, true, Variable::ARGUMENTS);
     arguments_shadow_ =
         new VariableProxy(Factory::arguments_shadow_symbol(), false, false);
     arguments_shadow_->BindTo(arguments_shadow);

src/scopes.h

@@ -47,7 +47,7 @@ class LocalsMap: public HashMap {
   virtual ~LocalsMap();
 
   Variable* Declare(Scope* scope, Handle<String> name, Variable::Mode mode,
-                    bool is_valid_LHS, bool is_this);
+                    bool is_valid_LHS, Variable::Kind kind);
 
   Variable* Lookup(Handle<String> name);
 };

src/variables.cc

@@ -140,12 +140,12 @@ Variable::Variable(Scope* scope,
                    Handle<String> name,
                    Mode mode,
                    bool is_valid_LHS,
-                   bool is_this)
+                   Kind kind)
   : scope_(scope),
     name_(name),
     mode_(mode),
     is_valid_LHS_(is_valid_LHS),
-    is_this_(is_this),
+    kind_(kind),
     local_if_not_shadowed_(NULL),
     is_accessed_from_inner_scope_(false),
     rewrite_(NULL) {

src/variables.h

@@ -137,6 +137,12 @@ class Variable: public ZoneObject {
                      // in a context
   };
 
+  enum Kind {
+    NORMAL,
+    THIS,
+    ARGUMENTS
+  };
+
   // Printing support
   static const char* Mode2String(Mode mode);
 
@@ -172,7 +178,8 @@ class Variable: public ZoneObject {
   }
 
   bool is_global() const;
-  bool is_this() const { return is_this_; }
+  bool is_this() const { return kind_ == THIS; }
+  bool is_arguments() const { return kind_ == ARGUMENTS; }
 
   Variable* local_if_not_shadowed() const {
     ASSERT(mode_ == DYNAMIC_LOCAL && local_if_not_shadowed_ != NULL);
@@ -190,13 +197,13 @@ class Variable: public ZoneObject {
 
  private:
   Variable(Scope* scope, Handle<String> name, Mode mode, bool is_valid_LHS,
-      bool is_this);
+           Kind kind);
 
   Scope* scope_;
   Handle<String> name_;
   Mode mode_;
   bool is_valid_LHS_;
-  bool is_this_;
+  Kind kind_;
 
   Variable* local_if_not_shadowed_;
 

src/x64/codegen-x64.cc

@@ -1257,6 +1257,10 @@ void CodeGenerator::VisitCall(Call* node) {
       // JavaScript example: 'object.foo(1, 2, 3)' or 'map["key"](1, 2, 3)'
       // ------------------------------------------------------------------
 
+      // TODO(X64): Consider optimizing Function.prototype.apply calls
+      // with arguments object. Requires lazy arguments allocation;
+      // see http://codereview.chromium.org/147075.
+
       // Push the name of the function and the receiver onto the stack.
       frame_->Push(literal->handle());
       Load(property->obj());

test/mjsunit/arguments-apply.js

+// Copyright 2009 the V8 project authors. All rights reserved.
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+//     * Redistributions of source code must retain the above copyright
+//       notice, this list of conditions and the following disclaimer.
+//     * Redistributions in binary form must reproduce the above
+//       copyright notice, this list of conditions and the following
+//       disclaimer in the documentation and/or other materials provided
+//       with the distribution.
+//     * Neither the name of Google Inc. nor the names of its
+//       contributors may be used to endorse or promote products derived
+//       from this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+function ReturnArguments() {
+  return arguments;
+}
+
+function ReturnReceiver() {
+  return this;
+}
+
+
+function Global() {
+  return ReturnArguments.apply(this, arguments);
+}
+
+assertEquals(0, Global().length);
+assertEquals(1, Global(1).length);
+assertEquals(2, Global(2)[0]);
+assertEquals(2, Global(3, 4).length);
+assertEquals(3, Global(3, 4)[0]);
+assertEquals(4, Global(3, 4)[1]);
+
+
+function Local() {
+  var object = { f: ReturnArguments };
+  return object.f.apply(this, arguments);
+}
+
+assertEquals(0, Local().length);
+assertEquals(1, Local(1).length);
+assertEquals(2, Local(2)[0]);
+assertEquals(2, Local(3, 4).length);
+assertEquals(3, Local(3, 4)[0]);
+assertEquals(4, Local(3, 4)[1]);
+
+
+function ShadowArguments() {
+  var arguments = [3, 4];
+  return ReturnArguments.apply(this, arguments);
+}
+
+assertEquals(2, ShadowArguments().length);
+assertEquals(3, ShadowArguments()[0]);
+assertEquals(4, ShadowArguments()[1]);
+
+
+function NonObjectReceiver(receiver) {
+  return ReturnReceiver.apply(receiver, arguments);
+}
+
+assertEquals(42, NonObjectReceiver(42));
+assertEquals("object", typeof NonObjectReceiver(42));
+assertTrue(NonObjectReceiver(42) instanceof Number);
+assertTrue(this === NonObjectReceiver(null));
+assertTrue(this === NonObjectReceiver(void 0));
+
+
+function ShadowApply() {
+  function f() { return 42; }
+  f.apply = function() { return 87; }
+  return f.apply(this, arguments);
+}
+
+assertEquals(87, ShadowApply());
+assertEquals(87, ShadowApply(1));
+assertEquals(87, ShadowApply(1, 2));
+
+
+function CallNonFunction() {
+  var object = { apply: Function.prototype.apply };
+  return object.apply(this, arguments);
+}
+
+assertThrows(CallNonFunction, TypeError);
+
+
+// Make sure that the stack after the apply optimization is
+// in a valid state.
+function SimpleStackCheck() {
+  var sentinel = 42;
+  var result = ReturnArguments.apply(this, arguments);
+  assertTrue(result != null);
+  assertEquals(42, sentinel);
+}
+
+SimpleStackCheck();
+
+
+function ShadowArgumentsWithConstant() {
+  var arguments = null;
+  return ReturnArguments.apply(this, arguments);
+}
+
+assertEquals(0, ShadowArgumentsWithConstant().length);
+assertEquals(0, ShadowArgumentsWithConstant(1).length);
+assertEquals(0, ShadowArgumentsWithConstant(1, 2).length);
+
+
+// Make sure we can deal with unfolding lots of arguments on the
+// stack even in the presence of the apply optimizations.
+var array = new Array(2048);
+assertEquals(2048, Global.apply(this, array).length);

test/mjsunit/arguments-lazy.js

+// Copyright 2009 the V8 project authors. All rights reserved.
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+//     * Redistributions of source code must retain the above copyright
+//       notice, this list of conditions and the following disclaimer.
+//     * Redistributions in binary form must reproduce the above
+//       copyright notice, this list of conditions and the following
+//       disclaimer in the documentation and/or other materials provided
+//       with the distribution.
+//     * Neither the name of Google Inc. nor the names of its
+//       contributors may be used to endorse or promote products derived
+//       from this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+// Make sure we don't allocate the arguments object over and
+// over again.
+function SharedLazyArguments() {
+  return arguments === arguments;
+}
+
+assertTrue(SharedLazyArguments());
+
+
+// Make sure that accessing arguments doesn't clobber any
+// local variables called arguments.
+function ArgumentsOverride(x) {
+  var arguments = 42;
+  x = x ? x : 0;
+  return x + arguments;
+}
+
+assertEquals(42, ArgumentsOverride());
+assertEquals(43, ArgumentsOverride(1));
+assertEquals(44, ArgumentsOverride(2,3));