Don't inline calls for not often called methods

This changes the JIT so we don't inline methods that aren't often called, allowing for more room in JIT'ted code for methods that are often called. It also changes to keep track of compiling explicitly instead of (ab)using the call_count for that. Resetting the call count had other effects such as disabling that method for future jitting that has adverse effects. On my system this shaves off around 1 - 2 seconds of a CI run.
ryoqun · Feb 24, 2013 · a787d83 · a787d83
1 parent 9f5349b
commit a787d83
Show file tree

Hide file tree

Showing 11 changed files with 65 additions and 32 deletions.
diff --git a/vm/builtin/compiledcode.cpp b/vm/builtin/compiledcode.cpp
@@ -254,11 +254,7 @@ namespace rubinius {
 
     // Must happen only on the first specialization
     if(!v->unspecialized) {
-      if(execute == specialized_executor) {
-        rubinius::bug("cant setup unspecialized from specialized");
-      }
-
-      v->unspecialized = execute;
+      v->unspecialized = v->fallback;
     }
 
     for(int i = 0; i < MachineCode::cMaxSpecializations; i++) {

diff --git a/vm/inline_cache.hpp b/vm/inline_cache.hpp
@@ -301,9 +301,10 @@ namespace rubinius {
       return NULL;
     }
 
-    Class* get_class(int idx) {
-       MethodCacheEntry* entry = cache_[idx].entry();
+    Class* get_class(int idx, int* hits) {
+      MethodCacheEntry* entry = cache_[idx].entry();
       if(entry) {
+        *hits = cache_[idx].hits();
         return entry->receiver_class();
       } else {
         return NULL;

diff --git a/vm/llvm/background_compile_request.hpp b/vm/llvm/background_compile_request.hpp
@@ -16,15 +16,17 @@ namespace rubinius {
     CompiledCode* method_;
     Object* extra_;
 
-    bool is_block_;
     utilities::thread::Condition* waiter_;
+    int hits_;
+    bool is_block_;
 
   public:
-    BackgroundCompileRequest(STATE, CompiledCode* code, Object* extra, bool is_block=false)
+    BackgroundCompileRequest(STATE, CompiledCode* code, Object* extra, int hits, bool is_block=false)
       : method_(code)
       , extra_(extra)
-      , is_block_(is_block)
       , waiter_(0)
+      , hits_(hits)
+      , is_block_(is_block)
     {}
 
     MachineCode* machine_code() {
@@ -51,6 +53,10 @@ namespace rubinius {
       return is_block_;
     }
 
+    int hits() {
+      return hits_;
+    }
+
     void set_waiter(utilities::thread::Condition* cond) {
       waiter_ = cond;
     }

diff --git a/vm/llvm/inline.cpp b/vm/llvm/inline.cpp
@@ -30,8 +30,9 @@ namespace rubinius {
 
   bool Inliner::consider_mono() {
     if(cache_->classes_seen() != 1) return false;
-    Class* klass = cache_->get_class(0);
-    return inline_for_class(klass);
+    int hits = 0;
+    Class* klass = cache_->get_class(0, &hits);
+    return inline_for_class(klass, hits);
   }
 
   bool Inliner::consider_poly() {
@@ -47,11 +48,12 @@ namespace rubinius {
     ops_.set_block(current);
 
     for(int i = 0; i < classes_seen; ++i) {
-      Class* klass = cache_->get_class(i);
+      int hits = 0;
+      Class* klass = cache_->get_class(i, &hits);
       // Fallback to the next for failure
       set_failure(fallback);
 
-      if(!inline_for_class(klass)) {
+      if(!inline_for_class(klass, hits)) {
         // If we fail to inline this, emit a send to the method
         Value* cache_const = ops_.b().CreateIntToPtr(
           ConstantInt::get(ops_.context()->IntPtrTy, (reinterpret_cast<uintptr_t>(cache_))),
@@ -104,7 +106,7 @@ namespace rubinius {
     return true;
   }
 
-  bool Inliner::inline_for_class(Class* klass) {
+  bool Inliner::inline_for_class(Class* klass, int hits) {
     if(!klass) return false;
 
     Module* defined_in = 0;
@@ -154,6 +156,8 @@ namespace rubinius {
 
         if(mcode->no_inline_p()) {
           decision = cInlineDisabled;
+        } else if(ops_.info().hits / 10 > hits) {
+          decision = cTooFewSends;
         } else {
           decision = policy->inline_p(mcode, opts);
         }
@@ -184,6 +188,9 @@ namespace rubinius {
                 ops_.llvm_state()->log() << " (block not allowed)";
               }
               break;
+            case cTooFewSends:
+              ops_.llvm_state()->log() << "too few sends: (" << hits << " / " << ops_.info().hits << ")";
+              break;
             default:
               ops_.llvm_state()->log() << "no policy";
             }
@@ -220,7 +227,7 @@ namespace rubinius {
         policy->increase_size(mcode);
         meth->add_inliner(ops_.llvm_state()->shared().om, ops_.root_method_info()->method());
 
-        inline_generic_method(klass, defined_in, code, mcode);
+        inline_generic_method(klass, defined_in, code, mcode, hits);
         return true;
       } else {
         if(ops_.llvm_state()->config().jit_inline_debug) {
@@ -546,14 +553,15 @@ namespace rubinius {
   }
 
   void Inliner::inline_generic_method(Class* klass, Module* defined_in,
-                                      CompiledCode* code, MachineCode* mcode) {
+                                      CompiledCode* code, MachineCode* mcode, int hits) {
     ctx_->enter_inline();
 
     check_recv(klass);
 
     JITMethodInfo info(ctx_, code, mcode);
 
     prime_info(info);
+    info.hits = hits;
 
     info.self_type = guarded_type_;
 

diff --git a/vm/llvm/inline.hpp b/vm/llvm/inline.hpp
@@ -143,11 +143,11 @@ namespace rubinius {
 
     bool consider_poly();
 
-    bool inline_for_class(Class* klass);
+    bool inline_for_class(Class* klass, int hits);
 
     void inline_block(JITInlineBlock* ib, Value* self);
 
-    void inline_generic_method(Class* klass, Module* mod, CompiledCode* code, MachineCode* mcode);
+    void inline_generic_method(Class* klass, Module* mod, CompiledCode* code, MachineCode* mcode, int hits);
 
     bool detect_trivial_method(MachineCode* mcode, CompiledCode* code = 0);
 

diff --git a/vm/llvm/inline_policy.hpp b/vm/llvm/inline_policy.hpp
@@ -117,7 +117,8 @@ namespace rubinius {
     cInline,
     cTooComplex,
     cTooBig,
-    cInlineDisabled
+    cInlineDisabled,
+    cTooFewSends
   };
 
   class InlinePolicy {

diff --git a/vm/llvm/jit_compiler.cpp b/vm/llvm/jit_compiler.cpp
@@ -143,13 +143,17 @@ namespace jit {
 
   void Compiler::compile(BackgroundCompileRequest* req) {
     if(req->is_block()) {
-      compile_block(req->method(), req->machine_code());
+      compile_block(req);
     } else {
       compile_method(req);
     }
   }
 
-  void Compiler::compile_block(CompiledCode* code, MachineCode* mcode) {
+  void Compiler::compile_block(BackgroundCompileRequest* req) {
+
+    CompiledCode* code = req->method();
+    MachineCode* mcode = req->machine_code();
+
     if(ctx_->llvm_state()->config().jit_inline_debug) {
 
       struct timeval tv;
@@ -175,6 +179,7 @@ namespace jit {
 
     JITMethodInfo info(ctx_, code, mcode);
     info.is_block = true;
+    info.hits = req->hits();
 
     ctx_->set_root(&info);
 
@@ -221,6 +226,7 @@ namespace jit {
 
     JITMethodInfo info(ctx_, code, code->machine_code());
     info.is_block = false;
+    info.hits = req->hits();
 
     if(Class* cls = req->receiver_class()) {
       info.set_self_class(cls);

diff --git a/vm/llvm/jit_compiler.hpp b/vm/llvm/jit_compiler.hpp
@@ -60,7 +60,7 @@ namespace jit {
 
     void compile(BackgroundCompileRequest* req);
     void compile_method(BackgroundCompileRequest* req);
-    void compile_block(CompiledCode* code, MachineCode* mcode);
+    void compile_block(BackgroundCompileRequest* req);
     void compile_builder(JITMethodInfo&, rubinius::jit::Builder&);
 
     void* function_pointer();

diff --git a/vm/llvm/method_info.hpp b/vm/llvm/method_info.hpp
@@ -49,6 +49,7 @@ namespace rubinius {
     InlinePolicy* inline_policy;
     llvm::BasicBlock* fin_block;
     int called_args;
+    int hits;
     JITStackArgs* stack_args;
 
     JITMethodInfo* root;

diff --git a/vm/llvm/state.cpp b/vm/llvm/state.cpp
@@ -361,6 +361,8 @@ namespace rubinius {
             req->method()->set_unspecialized(reinterpret_cast<executor>(func), rd);
           }
 
+          req->machine_code()->clear_compiling();
+
           // assert(req->method()->jit_data());
 
           ls_->end_method_update();
@@ -560,12 +562,17 @@ namespace rubinius {
       return;
     }
 
+    if(code->machine_code()->compiling_p()) {
+      return;
+    }
+
     // Don't do this because it prevents other class from heating
     // it up too!
-    code->machine_code()->call_count = -1;
+    int hits = code->machine_code()->call_count;
+    code->machine_code()->set_compiling();
 
     BackgroundCompileRequest* req =
-      new BackgroundCompileRequest(state, code, placement, is_block);
+      new BackgroundCompileRequest(state, code, placement, hits, is_block);
 
     queued_methods_++;
 
@@ -646,10 +653,6 @@ namespace rubinius {
       candidate->print_backtrace(state, 1);
     }
 
-    if(start && candidate->compiled_code != start) {
-      start->machine_code()->call_count = 0;
-    }
-
     if(candidate->compiled_code->machine_code()->call_count <= 1) {
       if(!start || start->machine_code()->jitted()) return;
       // Ignore it. compile this one.
@@ -828,8 +831,6 @@ namespace rubinius {
 
       // if(!next || cur->machine_code()->total > SMALL_METHOD_SIZE) return call_frame;
 
-      callee->compiled_code->machine_code()->call_count = 0;
-
       callee = call_frame;
       call_frame = prev;
     }

diff --git a/vm/machine_code.hpp b/vm/machine_code.hpp
@@ -32,7 +32,8 @@ namespace rubinius {
     static void** instructions;
 
     enum Flags {
-      eNoInline = 1 << 0
+      eNoInline  = 1 << 0,
+      eCompiling = 1 << 1
     };
 
     const static int cMaxSpecializations = 3;
@@ -105,6 +106,18 @@ namespace rubinius {
       return execute_status_ == eJITDisable;
     }
 
+    bool compiling_p() {
+      return (flags & eCompiling) == eCompiling;
+    }
+
+    void set_compiling() {
+      flags |= eCompiling;
+    }
+
+    void clear_compiling() {
+      flags &= ~eCompiling;
+    }
+
     void set_execute_status(ExecuteStatus s) {
       execute_status_ = s;
     }