Relax inlining limits for simple leaf functions.

src/ast.h

Index: src/ast.h
diff --git a/src/ast.h b/src/ast.h
index c2d9412a90b227e59885504ed79effd08ad8dcdc..88ab1cd5c2be4f380497ccfcf249576ca4e1adb9 100644
--- a/src/ast.h
+++ b/src/ast.h
@@ -167,7 +167,6 @@ class AstNode: public ZoneObject {
   // 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:
@@ -181,6 +180,11 @@ class AstNode: public ZoneObject {
     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.
@@ -441,6 +445,7 @@ class IterationStatement: public BreakableStatement {
       : BreakableStatement(isolate, labels, TARGET_FOR_ANONYMOUS),
         body_(NULL),
         osr_entry_id_(GetNextId(isolate)) {
+    IncrementNonPrimitiveAstNodeCounter(isolate);
   }
 
   void Initialize(Statement* body) {
@@ -686,8 +691,10 @@ class ReturnStatement: public Statement {
 
 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)
 
@@ -754,6 +761,7 @@ class SwitchStatement: public BreakableStatement {
       : BreakableStatement(isolate, labels, TARGET_FOR_ANONYMOUS),
         tag_(NULL),
         cases_(NULL) {
+    IncrementNonPrimitiveAstNodeCounter(isolate);
   }
 
 
@@ -790,8 +798,7 @@ class IfStatement: public 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)
 
@@ -846,8 +853,7 @@ class TryStatement: public Statement {
   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;
@@ -869,7 +875,8 @@ class TryStatement: public Statement {
 
 class TryCatchStatement: public TryStatement {
  public:
-  TryCatchStatement(int index,
+  TryCatchStatement(Isolate* isolate,
+                    int index,
                     Block* try_block,
                     Scope* scope,
                     Variable* variable,
@@ -878,6 +885,7 @@ class TryCatchStatement: public TryStatement {
         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)
@@ -896,9 +904,14 @@ class TryCatchStatement: public TryStatement {
 
 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)
 
@@ -1242,6 +1255,7 @@ class Call: public Expression {
         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)
@@ -1297,7 +1311,9 @@ class CallNew: public Expression {
       : 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)
 
@@ -1327,7 +1343,9 @@ class CallRuntime: public Expression {
       : 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)
 
@@ -1665,7 +1683,9 @@ class FunctionLiteral: public Expression {
                   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),
@@ -1676,7 +1696,9 @@ class FunctionLiteral: public Expression {
         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) |
@@ -1684,6 +1706,7 @@ class FunctionLiteral: public Expression {
         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)
@@ -1731,6 +1754,14 @@ class FunctionLiteral: public Expression {
     return HasDuplicateParameters::decode(bitfield_);
   }
 
+  int ast_node_count() {
+    return ast_node_count_;
+  }
+
+  bool is_primitive() {
+    return is_primitive_;
+  }
+
  private:
   Handle<String> name_;
   Scope* scope_;
@@ -1744,6 +1775,9 @@ class FunctionLiteral: public Expression {
   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> {};
@@ -1758,7 +1792,9 @@ class SharedFunctionInfoLiteral: public Expression {
   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)
 
@@ -1774,7 +1810,9 @@ class SharedFunctionInfoLiteral: public Expression {
 
 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;
 };