Conversation
|
|
||
| if (jittedMethod != null) { | ||
| return jittedMethod.call(context, self, clazz, name, args, block); | ||
| if (actualMethod != null) { |
There was a problem hiding this comment.
used to be one volatile field read, now its 2 right?
not sure it matters - but I think Java 8 still doesn't re-arrange volatile reads
There was a problem hiding this comment.
@kares you're right, even though this isn't really contended it's still better to copy. Sec adding the copy back :)
There was a problem hiding this comment.
Fixed :)
original-brownbear
force-pushed
the
faster-method-ir
branch
from
9782fad to
6179a95
Compare
|
looking good, always wondered whether we actually need to care about volatile semantics around call-count |
|
@kares yea I think |
3 participants
One performance improvement and one potential bugfix here:
Performance improvement:
DynamicMethodBoxadds a needless level of indirection. Moved the fields directly intoMixedModeIRMethodactualMethodfornullcheck and invocation. I think this is fine, there is no place in the code that could possibly set anullhere as far as I can see, right?Bugfix:
volatile int callCountis not safe for use as a counter here I think. It is incremented in a synchronized block (box.callCount++) in the jit method, but can also be set without synchronization fromsetCallCount(int callCount).callCountWith indy on, this gets me from
0.055 ops/ns(master) to0.061ops/ns(this branch) for the JMHorg.jruby.benchmark.JavaInterfaceBenchmark#benchHalfRubyVersion:)