Skip to content

Commit

Permalink
Merge branch 'jit-conc-fixes'
Browse files Browse the repository at this point in the history
@headius
headius committed Aug 25, 2017
2 parents 5d57d83 + a0892f6 commit 38e16f8
Showing 4 changed files with 133 additions and 114 deletions.
79 changes: 42 additions & 37 deletions core/src/main/java/org/jruby/compiler/BlockJITTask.java
View file
Original file line number Diff line number Diff line change
@@ -47,52 +47,57 @@ public BlockJITTask(JITCompiler jitCompiler, MixedModeIRBlockBody body, String c
}

public void run() {
try {
String key = SexpMaker.sha1(body.getIRScope());
JVMVisitor visitor = new JVMVisitor(jitCompiler.runtime);
BlockJITClassGenerator generator = new BlockJITClassGenerator(className, methodName, key, jitCompiler.runtime, body, visitor);
// We synchronize against the JITCompiler object so at most one code body will jit at once in a given runtime.
// This works around unsolved concurrency issues within the process of preparing and jitting the IR.
// See #4739 for a reproduction script that produced various errors without this.
synchronized (jitCompiler) {
try {
String key = SexpMaker.sha1(body.getIRScope());
JVMVisitor visitor = new JVMVisitor(jitCompiler.runtime);
BlockJITClassGenerator generator = new BlockJITClassGenerator(className, methodName, key, jitCompiler.runtime, body, visitor);

JVMVisitorMethodContext context = new JVMVisitorMethodContext();
generator.compile(context);
JVMVisitorMethodContext context = new JVMVisitorMethodContext();
generator.compile(context);

// FIXME: reinstate active bytecode size check
// At this point we still need to reinstate the bytecode size check, to ensure we're not loading code
// that's so big that JVMs won't even try to compile it. Removed the check because with the new IR JIT
// bytecode counts often include all nested scopes, even if they'd be different methods. We need a new
// mechanism of getting all body sizes.
Class sourceClass = visitor.defineFromBytecode(body.getIRScope(), generator.bytecode(), new OneShotClassLoader(jitCompiler.runtime.getJRubyClassLoader()));
// FIXME: reinstate active bytecode size check
// At this point we still need to reinstate the bytecode size check, to ensure we're not loading code
// that's so big that JVMs won't even try to compile it. Removed the check because with the new IR JIT
// bytecode counts often include all nested scopes, even if they'd be different methods. We need a new
// mechanism of getting all body sizes.
Class sourceClass = visitor.defineFromBytecode(body.getIRScope(), generator.bytecode(), new OneShotClassLoader(jitCompiler.runtime.getJRubyClassLoader()));

if (sourceClass == null) {
// class could not be found nor generated; give up on JIT and bail out
jitCompiler.counts.failCount.incrementAndGet();
return;
} else {
generator.updateCounters(jitCompiler.counts, body.ensureInstrsReady());
}
if (sourceClass == null) {
// class could not be found nor generated; give up on JIT and bail out
jitCompiler.counts.failCount.incrementAndGet();
return;
} else {
generator.updateCounters(jitCompiler.counts, body.ensureInstrsReady());
}

// successfully got back a jitted body
// successfully got back a jitted body

if (jitCompiler.config.isJitLogging()) {
JITCompiler.log(body.getImplementationClass(), body.getFile(), body.getLine(), className + "." + methodName, "done jitting");
}
String jittedName = context.getVariableName();

String jittedName = context.getVariableName();
// blocks only have variable-arity
body.completeBuild(
new CompiledIRBlockBody(
JITCompiler.PUBLIC_LOOKUP.findStatic(sourceClass, jittedName, JVMVisitor.CLOSURE_SIGNATURE.type()),
body.getIRScope(),
((IRClosure) body.getIRScope()).getSignature().encode()));

// blocks only have variable-arity
body.completeBuild(
new CompiledIRBlockBody(
JITCompiler.PUBLIC_LOOKUP.findStatic(sourceClass, jittedName, JVMVisitor.CLOSURE_SIGNATURE.type()),
body.getIRScope(),
((IRClosure) body.getIRScope()).getSignature().encode()));
} catch (Throwable t) {
if (jitCompiler.config.isJitLogging()) {
JITCompiler.log(body.getImplementationClass(), body.getFile(), body.getLine(), className + "." + methodName, "Could not compile; passes run: " + body.getIRScope().getExecutedPasses(), t.getMessage());
if (jitCompiler.config.isJitLoggingVerbose()) {
t.printStackTrace();
if (jitCompiler.config.isJitLogging()) {
JITCompiler.log(body.getImplementationClass(), body.getFile(), body.getLine(), className + "." + methodName, "done jitting");
}
} catch (Throwable t) {
if (jitCompiler.config.isJitLogging()) {
JITCompiler.log(body.getImplementationClass(), body.getFile(), body.getLine(), className + "." + methodName, "Could not compile; passes run: " + body.getIRScope().getExecutedPasses(), t.getMessage());
if (jitCompiler.config.isJitLoggingVerbose()) {
t.printStackTrace();
}
}
}

jitCompiler.counts.failCount.incrementAndGet();
jitCompiler.counts.failCount.incrementAndGet();
}
}
}
}
153 changes: 79 additions & 74 deletions core/src/main/java/org/jruby/compiler/MethodJITTask.java
View file
Original file line number Diff line number Diff line change
@@ -54,104 +54,109 @@ public MethodJITTask(JITCompiler jitCompiler, MixedModeIRMethod method, String c
}

public void run() {
try {
// Check if the method has been explicitly excluded
if (jitCompiler.config.getExcludedMethods().size() > 0) {
String excludeModuleName = className;
if (method.getImplementationClass().getMethodLocation().isSingleton()) {
IRubyObject possibleRealClass = ((MetaClass) method.getImplementationClass()).getAttached();
if (possibleRealClass instanceof RubyModule) {
excludeModuleName = "Meta:" + ((RubyModule) possibleRealClass).getName();
// We synchronize against the JITCompiler object so at most one code body will jit at once in a given runtime.
// This works around unsolved concurrency issues within the process of preparing and jitting the IR.
// See #4739 for a reproduction script that produced various errors without this.
synchronized (jitCompiler) {
try {
// Check if the method has been explicitly excluded
if (jitCompiler.config.getExcludedMethods().size() > 0) {
String excludeModuleName = className;
if (method.getImplementationClass().getMethodLocation().isSingleton()) {
IRubyObject possibleRealClass = ((MetaClass) method.getImplementationClass()).getAttached();
if (possibleRealClass instanceof RubyModule) {
excludeModuleName = "Meta:" + ((RubyModule) possibleRealClass).getName();
}
}
}

if ((jitCompiler.config.getExcludedMethods().contains(excludeModuleName)
|| jitCompiler.config.getExcludedMethods().contains(excludeModuleName + '#' + methodName)
|| jitCompiler.config.getExcludedMethods().contains(methodName))) {
method.setCallCount(-1);
if ((jitCompiler.config.getExcludedMethods().contains(excludeModuleName)
|| jitCompiler.config.getExcludedMethods().contains(excludeModuleName + '#' + methodName)
|| jitCompiler.config.getExcludedMethods().contains(methodName))) {
method.setCallCount(-1);

if (jitCompiler.config.isJitLogging()) {
JITCompiler.log(method.getImplementationClass(), method.getFile(), method.getLine(), methodName, "skipping method: " + excludeModuleName + '#' + methodName);
if (jitCompiler.config.isJitLogging()) {
JITCompiler.log(method.getImplementationClass(), method.getFile(), method.getLine(), methodName, "skipping method: " + excludeModuleName + '#' + methodName);
}
return;
}
return;
}
}

String key = SexpMaker.sha1(method.getIRScope());
Ruby runtime = jitCompiler.runtime;
JVMVisitor visitor = new JVMVisitor(runtime);
MethodJITClassGenerator generator = new MethodJITClassGenerator(className, methodName, key, runtime, method, visitor);
String key = SexpMaker.sha1(method.getIRScope());
Ruby runtime = jitCompiler.runtime;
JVMVisitor visitor = new JVMVisitor(runtime);
MethodJITClassGenerator generator = new MethodJITClassGenerator(className, methodName, key, runtime, method, visitor);

JVMVisitorMethodContext context = new JVMVisitorMethodContext();
generator.compile(context);
JVMVisitorMethodContext context = new JVMVisitorMethodContext();
generator.compile(context);

// FIXME: reinstate active bytecode size check
// At this point we still need to reinstate the bytecode size check, to ensure we're not loading code
// that's so big that JVMs won't even try to compile it. Removed the check because with the new IR JIT
// bytecode counts often include all nested scopes, even if they'd be different methods. We need a new
// mechanism of getting all method sizes.
Class sourceClass = visitor.defineFromBytecode(method.getIRScope(), generator.bytecode(), new OneShotClassLoader(runtime.getJRubyClassLoader()));
// FIXME: reinstate active bytecode size check
// At this point we still need to reinstate the bytecode size check, to ensure we're not loading code
// that's so big that JVMs won't even try to compile it. Removed the check because with the new IR JIT
// bytecode counts often include all nested scopes, even if they'd be different methods. We need a new
// mechanism of getting all method sizes.
Class sourceClass = visitor.defineFromBytecode(method.getIRScope(), generator.bytecode(), new OneShotClassLoader(runtime.getJRubyClassLoader()));

if (sourceClass == null) {
// class could not be found nor generated; give up on JIT and bail out
jitCompiler.counts.failCount.incrementAndGet();
return;
} else {
generator.updateCounters(jitCompiler.counts, method.ensureInstrsReady());
}
if (sourceClass == null) {
// class could not be found nor generated; give up on JIT and bail out
jitCompiler.counts.failCount.incrementAndGet();
return;
} else {
generator.updateCounters(jitCompiler.counts, method.ensureInstrsReady());
}

// successfully got back a jitted method
long methodCount = jitCompiler.counts.successCount.incrementAndGet();
// successfully got back a jitted method
long methodCount = jitCompiler.counts.successCount.incrementAndGet();

// logEvery n methods based on configuration
if (jitCompiler.config.getJitLogEvery() > 0) {
if (methodCount % jitCompiler.config.getJitLogEvery() == 0) {
JITCompiler.log(method.getImplementationClass(), method.getFile(), method.getLine(), methodName, "live compiled methods: " + methodCount);
// logEvery n methods based on configuration
if (jitCompiler.config.getJitLogEvery() > 0) {
if (methodCount % jitCompiler.config.getJitLogEvery() == 0) {
JITCompiler.log(method.getImplementationClass(), method.getFile(), method.getLine(), methodName, "live compiled methods: " + methodCount);
}
}
}

if (jitCompiler.config.isJitLogging()) {
JITCompiler.log(method.getImplementationClass(), method.getFile(), method.getLine(), className + '.' + methodName, "done jitting");
}
if (jitCompiler.config.isJitLogging()) {
JITCompiler.log(method.getImplementationClass(), method.getFile(), method.getLine(), className + '.' + methodName, "done jitting");
}

String variableName = context.getVariableName();
MethodHandle variable = JITCompiler.PUBLIC_LOOKUP.findStatic(sourceClass, variableName, context.getNativeSignature(-1));
IntHashMap<MethodType> signatures = context.getNativeSignaturesExceptVariable();

String variableName = context.getVariableName();
MethodHandle variable = JITCompiler.PUBLIC_LOOKUP.findStatic(sourceClass, variableName, context.getNativeSignature(-1));
IntHashMap<MethodType> signatures = context.getNativeSignaturesExceptVariable();

if (signatures.size() == 0) {
// only variable-arity
method.completeBuild(
new CompiledIRMethod(
variable,
method.getIRScope(),
method.getVisibility(),
method.getImplementationClass(),
method.getIRScope().receivesKeywordArgs()));

} else {
// also specific-arity
for (IntHashMap.Entry<MethodType> entry : signatures.entrySet()) {
if (signatures.size() == 0) {
// only variable-arity
method.completeBuild(
new CompiledIRMethod(
variable,
JITCompiler.PUBLIC_LOOKUP.findStatic(sourceClass, context.getSpecificName(), entry.getValue()),
entry.getKey(),
method.getIRScope(),
method.getVisibility(),
method.getImplementationClass(),
method.getIRScope().receivesKeywordArgs()));
break; // FIXME: only supports one arity

} else {
// also specific-arity
for (IntHashMap.Entry<MethodType> entry : signatures.entrySet()) {
method.completeBuild(
new CompiledIRMethod(
variable,
JITCompiler.PUBLIC_LOOKUP.findStatic(sourceClass, context.getSpecificName(), entry.getValue()),
entry.getKey(),
method.getIRScope(),
method.getVisibility(),
method.getImplementationClass(),
method.getIRScope().receivesKeywordArgs()));
break; // FIXME: only supports one arity
}
}
}
} catch (Throwable t) {
if (jitCompiler.config.isJitLogging()) {
JITCompiler.log(method.getImplementationClass(), method.getFile(), method.getLine(), className + '.' + methodName, "Could not compile; passes run: " + method.getIRScope().getExecutedPasses(), t.getMessage());
if (jitCompiler.config.isJitLoggingVerbose()) {
t.printStackTrace();
} catch (Throwable t) {
if (jitCompiler.config.isJitLogging()) {
JITCompiler.log(method.getImplementationClass(), method.getFile(), method.getLine(), className + '.' + methodName, "Could not compile; passes run: " + method.getIRScope().getExecutedPasses(), t.getMessage());
if (jitCompiler.config.isJitLoggingVerbose()) {
t.printStackTrace();
}
}
}

jitCompiler.counts.failCount.incrementAndGet();
jitCompiler.counts.failCount.incrementAndGet();
}
}
}
}
12 changes: 9 additions & 3 deletions core/src/main/java/org/jruby/parser/StaticScope.java
View file
Original file line number Diff line number Diff line change
@@ -117,7 +117,7 @@ public class StaticScope implements Serializable {

private RubyModule overlayModule;

private MethodHandle constructor;
private volatile MethodHandle constructor;

public enum Type {
LOCAL, BLOCK, EVAL;
@@ -189,16 +189,22 @@ public DynamicScope construct(DynamicScope parent) {
}
}

private MethodHandle acquireConstructor() {
MethodHandle constructor;
private synchronized MethodHandle acquireConstructor() {
// check again
MethodHandle constructor = this.constructor;

if (constructor != null) return constructor;

int numberOfVariables = getNumberOfVariables();

if (numberOfVariables > MAX_SPECIALIZED_SIZE) {
constructor = ManyVarsDynamicScope.CONSTRUCTOR;
} else {
constructor = DynamicScopeGenerator.generate(numberOfVariables);
}

this.constructor = constructor;

return constructor;
}

3 changes: 3 additions & 0 deletions core/src/main/java/org/jruby/runtime/MixedModeIRBlockBody.java
View file
Original file line number Diff line number Diff line change
@@ -178,6 +178,9 @@ protected void promoteToFullBuild(ThreadContext context) {
}

synchronized (this) {
// check call count again
if (callCount < 0) return;

if (callCount++ >= Options.JIT_THRESHOLD.load()) {
callCount = -1;
context.runtime.getJITCompiler().buildThresholdReached(context, this);

0 comments on commit 38e16f8

Please sign in to comment.