Skip to content

Commit

Permalink
Faster MixedModeIRMethod (#4786)
Browse files Browse the repository at this point in the history
One performance improvement and one potential bugfix here:

Performance improvement:

* `DynamicMethodBox` adds a needless level of indirection. Moved the fields directly into `MixedModeIRMethod`
* Stopped making a copy of `actualMethod` for `null` check and invocation. I think this is fine, there is no place in the code that could possibly set a `null` here as far as I can see, right?

Bugfix:

* `volatile int callCount` is 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 from `setCallCount(int callCount)`.
  * Fixed by synchronizing all write access to `callCount`
original-brownbear authored and kares committed Sep 17, 2017
1 parent 7a6eb69 commit c07da80
Showing 1 changed file with 33 additions and 39 deletions.
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
package org.jruby.internal.runtime.methods;

import java.io.ByteArrayOutputStream;
import org.jruby.MetaClass;
import org.jruby.RubyClass;
import org.jruby.RubyModule;
import org.jruby.compiler.Compilable;
import org.jruby.internal.runtime.AbstractIRMethod;
import org.jruby.ir.*;
import org.jruby.ir.IRMethod;
import org.jruby.ir.IRScope;
import org.jruby.ir.interpreter.InterpreterContext;
import org.jruby.ir.persistence.IRDumper;
import org.jruby.ir.runtime.IRRuntimeHelpers;
@@ -18,19 +20,13 @@
import org.jruby.util.log.Logger;
import org.jruby.util.log.LoggerFactory;

import java.io.ByteArrayOutputStream;

public class MixedModeIRMethod extends AbstractIRMethod implements Compilable<DynamicMethod> {
private static final Logger LOG = LoggerFactory.getLogger(MixedModeIRMethod.class);

private boolean displayedCFG = false; // FIXME: Remove when we find nicer way of logging CFG

protected static class DynamicMethodBox {
public volatile DynamicMethod actualMethod;
public volatile int callCount = 0;
}

protected DynamicMethodBox box = new DynamicMethodBox();
private volatile int callCount = 0;
private volatile DynamicMethod actualMethod;

public MixedModeIRMethod(IRScope method, Visibility visibility, RubyModule implementationClass) {
super(method, visibility, implementationClass);
@@ -39,12 +35,12 @@ public MixedModeIRMethod(IRScope method, Visibility visibility, RubyModule imple
// disable JIT if JIT is disabled
if (!implementationClass.getRuntime().getInstanceConfig().getCompileMode().shouldJIT() ||
Options.JIT_THRESHOLD.load() < 0) {
this.box.callCount = -1;
callCount = -1;
}
}

public DynamicMethod getActualMethod() {
return box.actualMethod;
return actualMethod;
}

protected void post(InterpreterContext ic, ThreadContext context) {
@@ -85,10 +81,9 @@ public InterpreterContext ensureInstrsReady() {
public IRubyObject call(ThreadContext context, IRubyObject self, RubyModule clazz, String name, IRubyObject[] args, Block block) {
if (IRRuntimeHelpers.isDebug()) doDebug();

DynamicMethodBox box = this.box;
if (box.callCount >= 0) tryJit(context, box);
DynamicMethod jittedMethod = box.actualMethod;
if (callCount >= 0) tryJit(context);

DynamicMethod jittedMethod = actualMethod;
if (jittedMethod != null) {
return jittedMethod.call(context, self, clazz, name, args, block);
} else {
@@ -120,10 +115,9 @@ private IRubyObject INTERPRET_METHOD(ThreadContext context, InterpreterContext i
public IRubyObject call(ThreadContext context, IRubyObject self, RubyModule clazz, String name, Block block) {
if (IRRuntimeHelpers.isDebug()) doDebug();

DynamicMethodBox box = this.box;
if (box.callCount >= 0) tryJit(context, box);
DynamicMethod jittedMethod = box.actualMethod;
if (callCount >= 0) tryJit(context);

DynamicMethod jittedMethod = actualMethod;
if (jittedMethod != null) {
return jittedMethod.call(context, self, clazz, name, block);
} else {
@@ -155,10 +149,9 @@ private IRubyObject INTERPRET_METHOD(ThreadContext context, InterpreterContext i
public IRubyObject call(ThreadContext context, IRubyObject self, RubyModule clazz, String name, IRubyObject arg0, Block block) {
if (IRRuntimeHelpers.isDebug()) doDebug();

DynamicMethodBox box = this.box;
if (box.callCount >= 0) tryJit(context, box);
DynamicMethod jittedMethod = box.actualMethod;
if (callCount >= 0) tryJit(context);

DynamicMethod jittedMethod = actualMethod;
if (jittedMethod != null) {
return jittedMethod.call(context, self, clazz, name, arg0, block);
} else {
@@ -190,10 +183,9 @@ private IRubyObject INTERPRET_METHOD(ThreadContext context, InterpreterContext i
public IRubyObject call(ThreadContext context, IRubyObject self, RubyModule clazz, String name, IRubyObject arg0, IRubyObject arg1, Block block) {
if (IRRuntimeHelpers.isDebug()) doDebug();

DynamicMethodBox box = this.box;
if (box.callCount >= 0) tryJit(context, box);
DynamicMethod jittedMethod = box.actualMethod;
if (callCount >= 0) tryJit(context);

DynamicMethod jittedMethod = actualMethod;
if (jittedMethod != null) {
return jittedMethod.call(context, self, clazz, name, arg0, arg1, block);
} else {
@@ -224,11 +216,9 @@ private IRubyObject INTERPRET_METHOD(ThreadContext context, InterpreterContext i
@Override
public IRubyObject call(ThreadContext context, IRubyObject self, RubyModule clazz, String name, IRubyObject arg0, IRubyObject arg1, IRubyObject arg2, Block block) {
if (IRRuntimeHelpers.isDebug()) doDebug();
if (callCount >= 0) tryJit(context);

DynamicMethodBox box = this.box;
if (box.callCount >= 0) tryJit(context, box);
DynamicMethod jittedMethod = box.actualMethod;

DynamicMethod jittedMethod = actualMethod;
if (jittedMethod != null) {
return jittedMethod.call(context, self, clazz, name, arg0, arg1, arg2, block);
} else {
@@ -273,19 +263,20 @@ protected void doDebug() {

@Override
public void completeBuild(DynamicMethod newMethod) {
this.box.actualMethod = newMethod;
this.box.actualMethod.serialNumber = this.serialNumber;
this.box.callCount = -1;
actualMethod = newMethod;
newMethod.serialNumber = this.serialNumber;
setCallCount(-1);
getImplementationClass().invalidateCacheDescendants();
}

protected void tryJit(ThreadContext context, DynamicMethodBox box) {
if (context.runtime.isBooting() && !Options.JIT_KERNEL.load()) return; // don't JIT during runtime boot

synchronized (this) {
if (box.callCount >= 0) {
if (box.callCount++ >= Options.JIT_THRESHOLD.load()) {
context.runtime.getJITCompiler().buildThresholdReached(context, this);
private void tryJit(ThreadContext context) {
// don't JIT during runtime boot
if (!context.runtime.isBooting() && !Options.JIT_KERNEL.load()) {
synchronized (this) {
if (callCount >= 0) {
if (callCount++ >= Options.JIT_THRESHOLD.load()) {
context.runtime.getJITCompiler().buildThresholdReached(context, this);
}
}
}
}
@@ -314,12 +305,15 @@ public String getClassName(ThreadContext context) {
@Override
public DynamicMethod dup() {
MixedModeIRMethod x = (MixedModeIRMethod) super.dup();
x.box = box;
x.callCount = callCount;
x.actualMethod = actualMethod;

return x;
}

public void setCallCount(int callCount) {
box.callCount = callCount;
synchronized (this) {
this.callCount = callCount;
}
}
}

0 comments on commit c07da80

Please sign in to comment.