Relax inlining limits for simple leaf functions.

src/ast.h

 // Copyright 2011 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
@@ skipping 149 matching lines @@
   virtual Statement* AsStatement() { return NULL; }
   virtual Expression* AsExpression() { return NULL; }
   virtual TargetCollector* AsTargetCollector() { return NULL; }
   virtual BreakableStatement* AsBreakableStatement() { return NULL; }
   virtual IterationStatement* AsIterationStatement() { return NULL; }
   virtual MaterializedLiteral* AsMaterializedLiteral() { return NULL; }
 
   // True if the node is simple enough for us to inline calls containing it.
   virtual bool IsInlineable() const = 0;
 
-  static int Count() { return Isolate::Current()->ast_node_count(); }
   static void ResetIds() { Isolate::Current()->set_ast_node_id(0); }
 
  protected:
   static unsigned GetNextId(Isolate* isolate) {
     return ReserveIdRange(isolate, 1);
   }
 
   static unsigned ReserveIdRange(Isolate* isolate, int n) {
     unsigned tmp = isolate->ast_node_id();
     isolate->set_ast_node_id(tmp + n);
     return tmp;
   }
 
+  void IncrementNonPrimitiveAstNodeCounter(Isolate* isolate) {
+    unsigned tmp = isolate->non_primitive_ast_node_count();
+    isolate->set_non_primitive_ast_node_count(tmp + 1);
+  }
+
  private:
   // Hidden to prevent accidental usage. It would have to load the
   // current zone from the TLS.
   void* operator new(size_t size);
 
   friend class CaseClause;  // Generates AST IDs.
 };
 
 
 class Statement: public AstNode {
@@ skipping 240 matching lines @@
   virtual int StackCheckId() const = 0;
 
   // Code generation
   Label* continue_target()  { return &continue_target_; }
 
  protected:
   IterationStatement(Isolate* isolate, ZoneStringList* labels)
       : BreakableStatement(isolate, labels, TARGET_FOR_ANONYMOUS),
         body_(NULL),
         osr_entry_id_(GetNextId(isolate)) {
+    IncrementNonPrimitiveAstNodeCounter(isolate);
   }
 
   void Initialize(Statement* body) {
     body_ = body;
   }
 
  private:
   Statement* body_;
   Label continue_target_;
   int osr_entry_id_;
@@ skipping 225 matching lines @@
   Expression* expression() const { return expression_; }
   virtual bool IsInlineable() const;
 
  private:
   Expression* expression_;
 };
 
 
 class WithStatement: public Statement {
  public:
-  WithStatement(Expression* expression, Statement* statement)
-      : expression_(expression), statement_(statement) { }
+  WithStatement(Isolate* isolate, Expression* expression, Statement* statement)

[Kevin Millikin (Chromium), 2011/11/28 12:11:59]

I think we need a crisp characterization of the things that we allow to be
inlined after the soft limit is reached.  Why do we explicitly forbid 'with'?

[ulan, 2011/11/29 14:06:52]

Done. Put the definition of "primitive" in the comment of
FunctionLiteral::is_primitive() and added a comment here.

+      : expression_(expression), statement_(statement) {
+    IncrementNonPrimitiveAstNodeCounter(isolate);
+  }
 
   DECLARE_NODE_TYPE(WithStatement)
 
   Expression* expression() const { return expression_; }
   Statement* statement() const { return statement_; }
 
   virtual bool IsInlineable() const;
 
  private:
   Expression* expression_;
@@ skipping 46 matching lines @@
   int entry_id_;
 };
 
 
 class SwitchStatement: public BreakableStatement {
  public:
   SwitchStatement(Isolate* isolate, ZoneStringList* labels)
       : BreakableStatement(isolate, labels, TARGET_FOR_ANONYMOUS),
         tag_(NULL),
         cases_(NULL) {
+    IncrementNonPrimitiveAstNodeCounter(isolate);
   }
 
 
   DECLARE_NODE_TYPE(SwitchStatement)
 
   void Initialize(Expression* tag, ZoneList<CaseClause*>* cases) {
     tag_ = tag;
     cases_ = cases;
   }
 
@@ skipping 16 matching lines @@
  public:
   IfStatement(Isolate* isolate,
               Expression* condition,
               Statement* then_statement,
               Statement* else_statement)
       : condition_(condition),
         then_statement_(then_statement),
         else_statement_(else_statement),
         if_id_(GetNextId(isolate)),
         then_id_(GetNextId(isolate)),
-        else_id_(GetNextId(isolate)) {
-  }
+        else_id_(GetNextId(isolate)) { }
 
   DECLARE_NODE_TYPE(IfStatement)
 
   virtual bool IsInlineable() const;
 
   bool HasThenStatement() const { return !then_statement()->IsEmpty(); }
   bool HasElseStatement() const { return !else_statement()->IsEmpty(); }
 
   Expression* condition() const { return condition_; }
   Statement* then_statement() const { return then_statement_; }
@@ skipping 34 matching lines @@
  private:
   ZoneList<Label*> targets_;
 };
 
 
 class TryStatement: public Statement {
  public:
   explicit TryStatement(int index, Block* try_block)
       : index_(index),
         try_block_(try_block),
-        escaping_targets_(NULL) {
-  }
+        escaping_targets_(NULL) { }
 
   void set_escaping_targets(ZoneList<Label*>* targets) {
     escaping_targets_ = targets;
   }
 
   int index() const { return index_; }
   Block* try_block() const { return try_block_; }
   ZoneList<Label*>* escaping_targets() const { return escaping_targets_; }
   virtual bool IsInlineable() const;
 
  private:
   // Unique (per-function) index of this handler.  This is not an AST ID.
   int index_;
 
   Block* try_block_;
   ZoneList<Label*>* escaping_targets_;
 };
 
 
 class TryCatchStatement: public TryStatement {
  public:
-  TryCatchStatement(int index,
+  TryCatchStatement(Isolate* isolate,
+                    int index,
                     Block* try_block,
                     Scope* scope,
                     Variable* variable,
                     Block* catch_block)
       : TryStatement(index, try_block),
         scope_(scope),
         variable_(variable),
         catch_block_(catch_block) {
+    IncrementNonPrimitiveAstNodeCounter(isolate);

[Kevin Millikin (Chromium), 2011/11/28 12:11:59]

It's inconsistent that the base class increments the counter for
IterationStatement, and the subclasses for TryCatchStatement and TryStatement. 
I wonder why that is?  

Is it so we can relax inlining restrictions on try/finally but not try/catch?

[ulan, 2011/11/29 14:06:52]

Done. Moved it to the base class.

   }
 
   DECLARE_NODE_TYPE(TryCatchStatement)
 
   Scope* scope() { return scope_; }
   Variable* variable() { return variable_; }
   Block* catch_block() const { return catch_block_; }
   virtual bool IsInlineable() const;
 
  private:
   Scope* scope_;
   Variable* variable_;
   Block* catch_block_;
 };
 
 
 class TryFinallyStatement: public TryStatement {
  public:
-  TryFinallyStatement(int index, Block* try_block, Block* finally_block)
+  TryFinallyStatement(Isolate* isolate,
+                      int index,
+                      Block* try_block,
+                      Block* finally_block)
       : TryStatement(index, try_block),
-        finally_block_(finally_block) { }
+        finally_block_(finally_block) {
+    IncrementNonPrimitiveAstNodeCounter(isolate);
+  }
 
   DECLARE_NODE_TYPE(TryFinallyStatement)
 
   Block* finally_block() const { return finally_block_; }
   virtual bool IsInlineable() const;
 
  private:
   Block* finally_block_;
 };
 
@@ skipping 323 matching lines @@
        Expression* expression,
        ZoneList<Expression*>* arguments,
        int pos)
       : Expression(isolate),
         expression_(expression),
         arguments_(arguments),
         pos_(pos),
         is_monomorphic_(false),
         check_type_(RECEIVER_MAP_CHECK),
         return_id_(GetNextId(isolate)) {
+    IncrementNonPrimitiveAstNodeCounter(isolate);

[Kevin Millikin (Chromium), 2011/11/28 12:11:59]

It's not obvious why we won't inline small functions containing calls after the
soft limit is reached?  Intuitively, it seems like it could be productive to
allow them.

Some comment is needed, otherwise someone will want to flip this bit and either
(1) have to repeat all your investigations that determined it was a bad idea or
(2) will break something that assumes it.

[ulan, 2011/11/29 14:06:52]

Added a comment saying that it is for performance. I didn't notice the
performance difference in this CL, but Jakob's CL  uses the fact that primitive
functions are leaf functions.

   }
 
   DECLARE_NODE_TYPE(Call)
 
   virtual bool IsInlineable() const;
 
   Expression* expression() const { return expression_; }
   ZoneList<Expression*>* arguments() const { return arguments_; }
   virtual int position() const { return pos_; }
 
@@ skipping 35 matching lines @@
 
 class CallNew: public Expression {
  public:
   CallNew(Isolate* isolate,
           Expression* expression,
           ZoneList<Expression*>* arguments,
           int pos)
       : Expression(isolate),
         expression_(expression),
         arguments_(arguments),
-        pos_(pos) { }
+        pos_(pos) {
+    IncrementNonPrimitiveAstNodeCounter(isolate);

[Kevin Millikin (Chromium), 2011/11/28 12:11:59]

Same comment as call.  It intuitively seems like it would be productive to
inline small functions containing 'new'.

[ulan, 2011/11/29 14:06:52]

Done.

+  }
 
   DECLARE_NODE_TYPE(CallNew)
 
   virtual bool IsInlineable() const;
 
   Expression* expression() const { return expression_; }
   ZoneList<Expression*>* arguments() const { return arguments_; }
   virtual int position() const { return pos_; }
 
  private:
   Expression* expression_;
   ZoneList<Expression*>* arguments_;
   int pos_;
 };
 
 
 // The CallRuntime class does not represent any official JavaScript
 // language construct. Instead it is used to call a C or JS function
 // with a set of arguments. This is used from the builtins that are
 // implemented in JavaScript (see "v8natives.js").
 class CallRuntime: public Expression {
  public:
   CallRuntime(Isolate* isolate,
               Handle<String> name,
               const Runtime::Function* function,
               ZoneList<Expression*>* arguments)
       : Expression(isolate),
         name_(name),
         function_(function),
-        arguments_(arguments) { }
+        arguments_(arguments) {
+    IncrementNonPrimitiveAstNodeCounter(isolate);

[Kevin Millikin (Chromium), 2011/11/28 12:11:59]

Same comment as Call and CallNew.  Why?

[ulan, 2011/11/29 14:06:52]

Done.

+  }
 
   DECLARE_NODE_TYPE(CallRuntime)
 
   virtual bool IsInlineable() const;
 
   Handle<String> name() const { return name_; }
   const Runtime::Function* function() const { return function_; }
   ZoneList<Expression*>* arguments() const { return arguments_; }
   bool is_jsruntime() const { return function_ == NULL; }
 
@@ skipping 317 matching lines @@
                   Handle<String> name,
                   Scope* scope,
                   ZoneList<Statement*>* body,
                   int materialized_literal_count,
                   int expected_property_count,
                   int handler_count,
                   bool has_only_simple_this_property_assignments,
                   Handle<FixedArray> this_property_assignments,
                   int parameter_count,
                   Type type,
-                  bool has_duplicate_parameters)
+                  bool has_duplicate_parameters,
+                  int ast_node_count,
+                  bool is_primitive)
       : Expression(isolate),
         name_(name),
         scope_(scope),
         body_(body),
         this_property_assignments_(this_property_assignments),
         inferred_name_(isolate->factory()->empty_string()),
         materialized_literal_count_(materialized_literal_count),
         expected_property_count_(expected_property_count),
         handler_count_(handler_count),
         parameter_count_(parameter_count),
-        function_token_position_(RelocInfo::kNoPosition) {
+        function_token_position_(RelocInfo::kNoPosition),
+        ast_node_count_(ast_node_count),
+        is_primitive_(is_primitive) {
     bitfield_ =
         HasOnlySimpleThisPropertyAssignments::encode(
             has_only_simple_this_property_assignments) |
         IsExpression::encode(type != DECLARATION) |
         IsAnonymous::encode(type == ANONYMOUS_EXPRESSION) |
         Pretenure::encode(false) |
         HasDuplicateParameters::encode(has_duplicate_parameters);
+    IncrementNonPrimitiveAstNodeCounter(isolate);

[Kevin Millikin (Chromium), 2011/11/28 12:11:59]

Same question: why is a function literal non-primitive, but all other literals
are primitive?

[ulan, 2011/11/29 14:06:52]

We do not inline functions containing function literals and materialized
literals. Made materialized literals non-primitive as well.

   }
 
   DECLARE_NODE_TYPE(FunctionLiteral)
 
   Handle<String> name() const { return name_; }
   Scope* scope() const { return scope_; }
   ZoneList<Statement*>* body() const { return body_; }
   void set_function_token_position(int pos) { function_token_position_ = pos; }
   int function_token_position() const { return function_token_position_; }
   int start_position() const;
@@ skipping 27 matching lines @@
   }
 
   bool pretenure() { return Pretenure::decode(bitfield_); }
   void set_pretenure() { bitfield_ |= Pretenure::encode(true); }
   virtual bool IsInlineable() const;
 
   bool has_duplicate_parameters() {
     return HasDuplicateParameters::decode(bitfield_);
   }
 
+  int ast_node_count() {
+    return ast_node_count_;
+  }
+
+  bool is_primitive() {
+    return is_primitive_;
+  }
+
  private:
   Handle<String> name_;
   Scope* scope_;
   ZoneList<Statement*>* body_;
   Handle<FixedArray> this_property_assignments_;
   Handle<String> inferred_name_;
 
   int materialized_literal_count_;
   int expected_property_count_;
   int handler_count_;
   int parameter_count_;
   int function_token_position_;
 
+  int ast_node_count_;
+  bool is_primitive_;
+
   unsigned bitfield_;
   class HasOnlySimpleThisPropertyAssignments: public BitField<bool, 0, 1> {};
   class IsExpression: public BitField<bool, 1, 1> {};
   class IsAnonymous: public BitField<bool, 2, 1> {};
   class Pretenure: public BitField<bool, 3, 1> {};
   class HasDuplicateParameters: public BitField<bool, 4, 1> {};
 };
 
 
 class SharedFunctionInfoLiteral: public Expression {
  public:
   SharedFunctionInfoLiteral(
       Isolate* isolate,
       Handle<SharedFunctionInfo> shared_function_info)
-      : Expression(isolate), shared_function_info_(shared_function_info) { }
+      : Expression(isolate), shared_function_info_(shared_function_info) {
+    IncrementNonPrimitiveAstNodeCounter(isolate);
+  }
 
   DECLARE_NODE_TYPE(SharedFunctionInfoLiteral)
 
   Handle<SharedFunctionInfo> shared_function_info() const {
     return shared_function_info_;
   }
   virtual bool IsInlineable() const;
 
  private:
   Handle<SharedFunctionInfo> shared_function_info_;
 };
 
 
 class ThisFunction: public Expression {
  public:
-  explicit ThisFunction(Isolate* isolate) : Expression(isolate) {}
+  explicit ThisFunction(Isolate* isolate) : Expression(isolate) {
+    IncrementNonPrimitiveAstNodeCounter(isolate);

[Kevin Millikin (Chromium), 2011/11/28 12:11:59]

This effectively prevents inlining exactly named function expressions after the
soft limit is reached.  Again, it's not clear *why* we want to explicitly
prohibit that compared to functions defined by function declarations and
anonymous function expressions.

[ulan, 2011/11/29 14:06:52]

You're right. There is no reason to make such functions non-primitive. Removed
the line.

+  }
   DECLARE_NODE_TYPE(ThisFunction)
   virtual bool IsInlineable() const;
 };
 
 
 // ----------------------------------------------------------------------------
 // Regular expressions
 
 
 class RegExpVisitor BASE_EMBEDDED {
@@ skipping 388 matching lines @@
 
  private:
   Isolate* isolate_;
   bool stack_overflow_;
 };
 
 
 } }  // namespace v8::internal
 
 #endif  // V8_AST_H_