MINOR: Make CachingCallSite more JIT friendly #4753
Conversation
original-brownbear
force-pushed
the
cachingcallsite-improvements
branch
from
d78c100 to
4bef608
Compare
original-brownbear
force-pushed
the
cachingcallsite-improvements
branch
from
4bef608 to
a12e5f1
Compare
| return typeOk(this, incomingType); | ||
| } | ||
|
|
||
| public static final boolean typeOk(CacheEntry entry, RubyClass incomingType) { |
There was a problem hiding this comment.
you need to careful when removing methods on places such as these.
might be called from JIT-ed byte-code (so you're not seeing it show up as used)
There was a problem hiding this comment.
@kares yea, I learnt that today in some other spot already :) Questions (if you have a sec):
- Can I do anything besides running tests + full-text search to make sure I don't kill such a method accidentally?
- Am I correct to understand that the
@JITannotation should be put on methods which could be called dynamically like you described?
Thanks!
There was a problem hiding this comment.
Can I do anything besides running tests + full-text search to make sure I don't kill such a method accidentally?
usually just grep for the method-name string -> "typeOf" should be safe to find whether its used :)
Am I correct to understand that the @jit annotation should be put on methods which could be called dynamically like you described?
well yeah - have run into that as well ... lately.
but what I dislike is that now some places might have a dependency on the IR package (org.jruby.ir.JIT) -> should maybe get relocated somewhere else I guess ... not sure
haven't raised that concern yet -> you should chat with @enebo or @headius on IRC if it was meant to be somehow IR specific or not.
There was a problem hiding this comment.
@kares thanks!
|
... merging this piece so I do not have a conflict with my fast-opts call-site changes later |
Some improvements I found when profiling the red black benchmark ( + some trivialities along the way :)):
org.jruby.RubyModule#searchWithCache(java.lang.String, boolean)into hot and cold path methods to decrease the size of its callerorg.jruby.runtime.callsite.CachingCallSite#cacheAndCall(org.jruby.runtime.builtin.IRubyObject, org.jruby.RubyClass, org.jruby.runtime.Block, org.jruby.runtime.ThreadContext, org.jruby.runtime.builtin.IRubyObject, org.jruby.runtime.builtin.IRubyObject, org.jruby.runtime.builtin.IRubyObject, org.jruby.runtime.builtin.IRubyObject)org.jruby.runtime.callsite.CacheEntry#typeOkand removed itsstaticversion (thestaticversion really made no sense as it required having an instance to feed into it anyhow + the instance version compiles a little smaller)updateCache(entry);to save a byteboolean cacheAndGetBuiltin(RubyClass selfType, String methodName)staticsince it contains no reference tothisTrivial:
@override, where missing to improve readabilityCachingCallingSitefinalto improve readability (I make JIT converge to the final state a little faster I guess :D)privatethat were needlesslyprotectedResults:
2.2%faster for me, measured over 1k iterations6.5Minstead of6.8M