Skip to content
Permalink

Comparing changes

Choose two branches to see what’s changed or to start a new pull request. If you need to, you can also or learn more about diff comparisons.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also . Learn more about diff comparisons here.
base repository: jruby/jruby
Failed to load repositories. Confirm that selected base ref is valid, then try again.
Loading
base: 79bbf323fd39
Choose a base ref
...
head repository: jruby/jruby
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: 84265774f9a3
Choose a head ref
  • 2 commits
  • 9 files changed
  • 1 contributor

Commits on Apr 4, 2016

  1. searchConst will not be going back to more primitive impl ever so cle…

    …an up commented out code
    enebo committed Apr 4, 2016
    Copy the full SHA
    ccb7706 View commit details
  2. Hello new instr SearchModuleForConst. This instr is somewhat of a

    combo instr.  It used to be emitted as InheritanceSearch + BNE +
    ConstMissing instr but this ended up being undesirable for a few
    or reasons:
    
    1. number of overall instrs are larger for no benefit.  IR as it
    exists could not make use of the finer granularity and based on
    how many side-effecting things get called between constant
    lookups it was unlikely we will be able to simplify a scope to
    only lookup a constant once.  Also under the IR layer can change
    const resolution to use the same invokedynamic bindings for the
    same "constant" in a scope essentially gambling that all constants
    with the same name will end up referencing the same constant in
    a given IR scope (which is probably 99.99999% true -- prove this
    wrong! :) ).
    
    2. increases the size of the CFG for no benefit.  not as big
    of a deal...
    
    3. interpretation is a little bit faster.  we moved this logic
    into Java so we are no longer emulating if statement and doing
    less overall work.  I tried red black bench and saw a small
    but measurable increase.
    enebo committed Apr 4, 2016
    Copy the full SHA
    8426577 View commit details
53 changes: 13 additions & 40 deletions core/src/main/java/org/jruby/ir/IRBuilder.java
Original file line number Diff line number Diff line change
@@ -4,7 +4,6 @@
import org.jcodings.specific.USASCIIEncoding;
import org.jruby.Ruby;
import org.jruby.RubyInstanceConfig;
import org.jruby.RubyString;
import org.jruby.ast.*;
import org.jruby.ast.types.INameNode;
import org.jruby.compiler.NotCompilableException;
@@ -1337,52 +1336,26 @@ private Operand putConstantAssignment(OpAsgnConstDeclNode node, Operand value) {
return putConstant((Colon3Node) constNode, value);
}

private void genInheritanceSearchInstrs(Operand startingModule, Variable constVal, Label foundLabel, boolean noPrivateConstants, String name) {
addInstr(new InheritanceSearchConstInstr(constVal, startingModule, name, noPrivateConstants));
addInstr(BNEInstr.create(foundLabel, constVal, UndefinedValue.UNDEFINED));
addInstr(new ConstMissingInstr(constVal, startingModule, name, scope.maybeUsingRefinements()));
addInstr(new LabelInstr(foundLabel));
private Operand searchModuleForConst(Operand startingModule, String name) {
return addResultInstr(new SearchModuleForConstInstr(createTemporaryVariable(), startingModule, name, true));
}

private Operand searchConstInInheritanceHierarchy(Operand startingModule, String name) {
Variable constVal = createTemporaryVariable();
genInheritanceSearchInstrs(startingModule, constVal, getNewLabel(), true, name);
return constVal;
private Operand searchConst(String name) {
return addResultInstr(new SearchConstInstr(createTemporaryVariable(), name, startingSearchScope(), false));
}

private Operand searchConst(String name) {
final boolean noPrivateConstants = false;
Variable v = createTemporaryVariable();
/**
* SSS FIXME: Went back to a single instruction for now.
*
* Do not split search into lexical-search, inheritance-search, and const-missing instrs.
*
Label foundLabel = getNewLabel();
addInstr(new LexicalSearchConstInstr(v, startingSearchScope(s), name));
addInstr(BNEInstr.create(v, UndefinedValue.UNDEFINED, foundLabel));
genInheritanceSearchInstrs(s, findContainerModule(startingScope), v, foundLabel, noPrivateConstants, name);
**/
addInstr(new SearchConstInstr(v, name, startingSearchScope(), noPrivateConstants));
return v;
}

public Operand buildColon2(final Colon2Node iVisited) {
Node leftNode = iVisited.getLeftNode();
final String name = iVisited.getName();

// Colon2ImplicitNode
if (leftNode == null) return searchConst(name);

// Colon2ConstNode
// 1. Load the module first (lhs of node)
// 2. Then load the constant from the module
Operand module = build(leftNode);
return searchConstInInheritanceHierarchy(module, name);
public Operand buildColon2(final Colon2Node colon2) {
Node lhs = colon2.getLeftNode();

// Colon2ImplicitNode - (module|class) Foo. Weird, but it is a wrinkle of AST inheritance.
if (lhs == null) return searchConst(colon2.getName());

// Colon2ConstNode (Left::name)
return searchModuleForConst(build(lhs), colon2.getName());
}

public Operand buildColon3(Colon3Node node) {
return searchConstInInheritanceHierarchy(new ObjectClass(), node.getName());
return searchModuleForConst(new ObjectClass(), node.getName());
}

public Operand buildComplex(ComplexNode node) {
1 change: 1 addition & 0 deletions core/src/main/java/org/jruby/ir/IRVisitor.java
Original file line number Diff line number Diff line change
@@ -124,6 +124,7 @@ private void error(Object object) {
public void RuntimeHelperCall(RuntimeHelperCall runtimehelpercall) { error(runtimehelpercall); }
public void SaveBindingVisibilityInstr(SaveBindingVisibilityInstr instr) { error(instr); }
public void SearchConstInstr(SearchConstInstr searchconstinstr) { error(searchconstinstr); }
public void SearchModuleForConstInstr(SearchModuleForConstInstr searchconstinstr) { error(searchconstinstr); }
public void SetCapturedVarInstr(SetCapturedVarInstr instr) { error(instr); }
public void StoreLocalVarInstr(StoreLocalVarInstr storelocalvarinstr) { error(storelocalvarinstr); }
public void ThreadPollInstr(ThreadPollInstr threadpollinstr) { error(threadpollinstr); }
1 change: 1 addition & 0 deletions core/src/main/java/org/jruby/ir/Operation.java
Original file line number Diff line number Diff line change
@@ -120,6 +120,7 @@ public enum Operation {
INHERITANCE_SEARCH_CONST(OpFlags.f_can_raise_exception),
CONST_MISSING(OpFlags.f_can_raise_exception),
SEARCH_CONST(OpFlags.f_can_raise_exception),
SEARCH_MODULE_FOR_CONST(OpFlags.f_can_raise_exception),

GET_GLOBAL_VAR(OpFlags.f_is_load),
GET_FIELD(OpFlags.f_is_load),
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
package org.jruby.ir.instructions;

import org.jruby.Ruby;
import org.jruby.RubyModule;
import org.jruby.ir.IRVisitor;
import org.jruby.ir.Operation;
import org.jruby.ir.operands.Operand;
import org.jruby.ir.operands.UndefinedValue;
import org.jruby.ir.operands.Variable;
import org.jruby.ir.persistence.IRReaderDecoder;
import org.jruby.ir.persistence.IRWriterEncoder;
import org.jruby.ir.transformations.inlining.CloneInfo;
import org.jruby.parser.StaticScope;
import org.jruby.runtime.DynamicScope;
import org.jruby.runtime.ThreadContext;
import org.jruby.runtime.builtin.IRubyObject;
import org.jruby.runtime.opto.ConstantCache;
import org.jruby.runtime.opto.Invalidator;

/**
* Search for a constant within the current module. If it cannot find then
* call const_missing.
*/
public class SearchModuleForConstInstr extends OneOperandResultBaseInstr implements FixedArityInstr {
String constName;
private final boolean noPrivateConsts;

// Constant caching
private volatile transient ConstantCache cache;

public SearchModuleForConstInstr(Variable result, Operand currentModule, String constName, boolean noPrivateConsts) {
super(Operation.SEARCH_MODULE_FOR_CONST, result, currentModule);

this.constName = constName;
this.noPrivateConsts = noPrivateConsts;
}

public Operand getCurrentModule() {
return getOperand1();
}

public String getConstName() {
return constName;
}

public boolean isNoPrivateConsts() {
return noPrivateConsts;
}

@Override
public Instr clone(CloneInfo ii) {
return new InheritanceSearchConstInstr(ii.getRenamedVariable(result),
getCurrentModule().cloneForInlining(ii), constName, noPrivateConsts);
}

@Override
public String[] toStringNonOperandArgs() {
return new String[] { "name: " + constName, "no_priv: " + noPrivateConsts};
}

private Object cache(Ruby runtime, RubyModule module) {
Object constant = noPrivateConsts ? module.getConstantFromNoConstMissing(constName, false) : module.getConstantNoConstMissing(constName);
if (constant == null) {
constant = UndefinedValue.UNDEFINED;
} else {
// recache
Invalidator invalidator = runtime.getConstantInvalidator(constName);
cache = new ConstantCache((IRubyObject)constant, invalidator.getData(), invalidator, module.hashCode());
}
return constant;
}

@Override
public void encode(IRWriterEncoder e) {
super.encode(e);
e.encode(getCurrentModule());
e.encode(getConstName());
e.encode(isNoPrivateConsts());
}

public static SearchModuleForConstInstr decode(IRReaderDecoder d) {
return new SearchModuleForConstInstr(d.decodeVariable(), d.decodeOperand(), d.decodeString(), d.decodeBoolean());
}

@Override
public Object interpret(ThreadContext context, StaticScope currScope, DynamicScope currDynScope, IRubyObject self, Object[] temp) {
Object cmVal = getCurrentModule().retrieve(context, self, currScope, currDynScope, temp);

if (!(cmVal instanceof RubyModule)) throw context.runtime.newTypeError(cmVal + " is not a type/class");

RubyModule module = (RubyModule) cmVal;
ConstantCache cache = this.cache;
Object result = !ConstantCache.isCachedFrom(module, cache) ? cache(context.runtime, module) : cache.value;

if (result == UndefinedValue.UNDEFINED) {
result = module.callMethod(context, "const_missing", context.runtime.fastNewSymbol(constName));
}

return result;
}

@Override
public void visit(IRVisitor visitor) {
visitor.SearchModuleForConstInstr(this);
}
}
Original file line number Diff line number Diff line change
@@ -279,6 +279,7 @@ public Instr decodeInstr() {
case RETURN_OR_RETHROW_SAVED_EXC: return ReturnOrRethrowSavedExcInstr.decode(this);
case RUNTIME_HELPER: return RuntimeHelperCall.decode(this);
case SEARCH_CONST: return SearchConstInstr.decode(this);
case SEARCH_MODULE_FOR_CONST: return SearchModuleForConstInstr.decode(this);
case SET_CAPTURED_VAR: return SetCapturedVarInstr.decode(this);
case TRACE: return TraceInstr.decode(this);
case THREAD_POLL: return ThreadPollInstr.decode(this);
32 changes: 32 additions & 0 deletions core/src/main/java/org/jruby/ir/targets/ConstantLookupSite.java
Original file line number Diff line number Diff line change
@@ -98,6 +98,38 @@ public IRubyObject searchConst(ThreadContext context, StaticScope staticScope) {
return constant;
}

public IRubyObject searchModuleForConst(ThreadContext context, RubyModule module) {
// Lexical lookup
Ruby runtime = context.getRuntime();
IRubyObject constant = publicOnly ? module.getConstantFromNoConstMissing(name, false) : module.getConstantNoConstMissing(name);

// Call const_missing or cache
if (constant == null) {
return module.callMethod(context, "const_missing", context.runtime.fastNewSymbol(name));
}

SwitchPoint switchPoint = (SwitchPoint) runtime.getConstantInvalidator(name).getData();

// bind constant until invalidated
MethodHandle target = Binder.from(type())
.drop(0, 2)
.constant(constant);
MethodHandle fallback = getTarget();
if (fallback == null) {
fallback = Binder.from(type())
.insert(0, this)
.invokeVirtualQuiet(Bootstrap.LOOKUP, "searchConst");
}

setTarget(switchPoint.guardWithTest(target, fallback));

if (Options.INVOKEDYNAMIC_LOG_CONSTANTS.load()) {
LOG.info(name + "\tretrieved and cached from module " + module);// + " added to PIC" + extractSourceInfo(site));
}

return constant;
}

public IRubyObject inheritanceSearchConst(ThreadContext context, IRubyObject cmVal) {
Ruby runtime = context.runtime;
RubyModule module;
11 changes: 11 additions & 0 deletions core/src/main/java/org/jruby/ir/targets/IRBytecodeAdapter.java
Original file line number Diff line number Diff line change
@@ -466,6 +466,17 @@ public org.objectweb.asm.Label newLabel() {
*/
public abstract void searchConst(String name, boolean noPrivateConsts);


/**
* Lookup a constant from current module.
*
* Stack required: context, static scope
*
* @param name name of the constant
* @param noPrivateConsts whether to ignore private constants
*/
public abstract void searchModuleForConst(String name, boolean noPrivateConsts);

/**
* Lookup a constant from a given class or module.
*
Original file line number Diff line number Diff line change
@@ -715,6 +715,10 @@ public void searchConst(String name, boolean noPrivateConsts) {
adapter.invokedynamic("searchConst", sig(JVM.OBJECT, params(ThreadContext.class, StaticScope.class)), ConstantLookupSite.BOOTSTRAP, name, noPrivateConsts ? 1 : 0);
}

public void searchModuleForConst(String name, boolean noPrivateConsts) {
adapter.invokedynamic("searchModuleForConst", sig(JVM.OBJECT, params(ThreadContext.class, RubyModule.class)), ConstantLookupSite.BOOTSTRAP, name, noPrivateConsts ? 1 : 0);
}

public void inheritanceSearchConst(String name, boolean noPrivateConsts) {
adapter.invokedynamic("inheritanceSearchConst", sig(JVM.OBJECT, params(ThreadContext.class, IRubyObject.class)), ConstantLookupSite.BOOTSTRAP, name, noPrivateConsts ? 1 : 0);
}
8 changes: 8 additions & 0 deletions core/src/main/java/org/jruby/ir/targets/JVMVisitor.java
Original file line number Diff line number Diff line change
@@ -1918,6 +1918,14 @@ public void SearchConstInstr(SearchConstInstr searchconstinstr) {
jvmStoreLocal(searchconstinstr.getResult());
}

@Override
public void SearchModuleForConstInstr(SearchModuleForConstInstr instr) {
jvmMethod().loadContext();
visit(instr.getCurrentModule());

jvmMethod().searchModuleForConst(instr.getConstName(), instr.isNoPrivateConsts());
}

@Override
public void SetCapturedVarInstr(SetCapturedVarInstr instr) {
jvmMethod().loadContext();