Skip to content

Commit

Permalink
Transitional changes towards eliminating instrList from IRScope
Browse files Browse the repository at this point in the history
  • Loading branch information
@enebo
enebo committed Mar 3, 2015
1 parent 5ad4c5c commit dee6c77
Show file tree
Hide file tree
Showing 7 changed files with 51 additions and 43 deletions.
8 changes: 6 additions & 2 deletions core/src/main/java/org/jruby/ir/IRClosure.java
View file
Expand Up @@ -91,8 +91,12 @@ public IRClosure(IRManager manager, IRScope lexicalParent, int lineNumber, Stati
}

@Override
public InterpreterContext allocateInterpreterContext(Instr[] instructionList, boolean rebuild) {
return new ClosureInterpreterContext(this, instructionList, rebuild);
public InterpreterContext allocateInterpreterContext() {
InterpreterContext interpreterContext = new ClosureInterpreterContext(this, instrList);

instrList = null; // Once IC has this we are done with this structure forever more...

return interpreterContext;
}

public void setBeginEndBlock() {
Expand Down
9 changes: 6 additions & 3 deletions core/src/main/java/org/jruby/ir/IREvalScript.java
View file
Expand Up @@ -3,7 +3,6 @@
import java.util.ArrayList;
import java.util.List;
import org.jruby.EvalType;
import org.jruby.ir.instructions.Instr;
import org.jruby.ir.interpreter.BeginEndInterpreterContext;
import org.jruby.ir.interpreter.InterpreterContext;
import org.jruby.ir.operands.Label;
Expand Down Expand Up @@ -35,8 +34,12 @@ public IREvalScript(IRManager manager, IRScope lexicalParent, String fileName,
}

@Override
public InterpreterContext allocateInterpreterContext(Instr[] instructionList, boolean rebuild) {
return new BeginEndInterpreterContext(this, instructionList, rebuild);
public InterpreterContext allocateInterpreterContext() {
InterpreterContext interpreterContext = new BeginEndInterpreterContext(this, instrList);

instrList = null; // Once IC has this we are done with this structure forever more...

return interpreterContext;
}

@Override
Expand Down
37 changes: 13 additions & 24 deletions core/src/main/java/org/jruby/ir/IRScope.java
View file
Expand Up @@ -92,7 +92,7 @@ public abstract class IRScope implements ParseResult {
private StaticScope staticScope;

/** List of IR instructions for this method */
private List<Instr> instrList;
protected List<Instr> instrList;

/** Control flow graph representation of this method's instructions */
private CFG cfg;
Expand Down Expand Up @@ -447,7 +447,7 @@ public CFG buildCFG() {
}

CFG newCFG = new CFG(this);
newCFG.build(prepareBuildInstructions(getInstrs()));
newCFG.build(interpreterContext.getInstructions());
// Clear out instruction list after CFG has been built.
instrList = null;

Expand All @@ -456,19 +456,6 @@ public CFG buildCFG() {
return newCFG;
}

private Instr[] prepareBuildInstructions(List<Instr> instructions) {
int length = instructions.size();
Instr[] linearizedInstrArray = instructions.toArray(new Instr[length]);
for (int ipc = 0; ipc < length; ipc++) {
Instr i = linearizedInstrArray[ipc];
i.setIPC(ipc);

if (i instanceof LabelInstr) ((LabelInstr) i).getLabel().setTargetPC(ipc + 1);
}

return linearizedInstrArray;
}

protected void setCFG(CFG cfg) {
this.cfg = cfg;
}
Expand Down Expand Up @@ -631,8 +618,12 @@ protected void initScope(boolean jitMode) {
}

/** Make version specific to scope which needs it (e.g. Closure vs non-closure). */
public InterpreterContext allocateInterpreterContext(Instr[] instructionList, boolean rebuild) {
return new InterpreterContext(this, instructionList, rebuild);
public InterpreterContext allocateInterpreterContext() {
InterpreterContext interpreterContext = new InterpreterContext(this, instrList);

instrList = null;

return interpreterContext;
}

protected void cloneInstrs() {
Expand Down Expand Up @@ -663,15 +654,15 @@ protected void cloneInstrs(SimpleCloneInfo cloneInfo) {
/**
* Get an existing interpreter context or create a new one if it has not been made before
*/
public InterpreterContext acquireInterpreterContext() {
public synchronized InterpreterContext acquireInterpreterContext() {
// Try unsync access first before calling more expensive method for getting IC
// Also get reference in case second thread is trying to do the same thing at near same time
InterpreterContext ic = interpreterContext;

if (ic == null) { // Never been interp'd. Make simplest interpreter.
ic = prepareForBuildInterpretation();
} else if (ic.needsRebuilding()) { // Already have IC but IC says it is time to take it up a notch!
prepareForFullBuildInterpretation();
//prepareForFullBuildInterpretation();
}

return ic;
Expand All @@ -681,7 +672,7 @@ public InterpreterContext acquireInterpreterContext() {
* Called directly after IRBuild but before CFG is built.
*/
public synchronized InterpreterContext prepareForBuildInterpretation() {
interpreterContext = allocateInterpreterContext(prepareInstructions(), false);
interpreterContext = allocateInterpreterContext();

return interpreterContext;
}
Expand Down Expand Up @@ -1132,10 +1123,8 @@ public Operand[] getCallArgs() {
// We have two paths. eval and non-eval.
if (instrList == null) { // CFG already made. eval has zsuper and we walk back to some executing method/script
// FIXME: Need to verify this can never re-order recvs in a way to swap order to zsuper
for (BasicBlock bb: getCFG().getBasicBlocks()) {
for (Instr instr: bb.getInstrs()) {
extractCallOperands(callArgs, keywordArgs, instr);
}
for (Instr instr: interpreterContext.getInstructions()) {
extractCallOperands(callArgs, keywordArgs, instr);
}
} else { // common zsuper case. non-eval and at build time entirely.
for (Instr instr : getInstrs()) {
Expand Down
7 changes: 3 additions & 4 deletions core/src/main/java/org/jruby/ir/IRScriptBody.java
View file
@@ -1,6 +1,5 @@
package org.jruby.ir;

import org.jruby.ir.instructions.Instr;
import org.jruby.ir.interpreter.BeginEndInterpreterContext;
import org.jruby.ir.interpreter.InterpreterContext;
import org.jruby.parser.StaticScope;
Expand Down Expand Up @@ -31,8 +30,8 @@ public void setTopLevelBindingScope(DynamicScope tlbScope) {
}

@Override
public InterpreterContext allocateInterpreterContext(Instr[] instructionList, boolean rebuild) {
return new BeginEndInterpreterContext(this, instructionList, rebuild);
public InterpreterContext allocateInterpreterContext() {
return new BeginEndInterpreterContext(this, instrList);
}

@Override
Expand All @@ -53,7 +52,7 @@ public String toString() {
/* Record a begin block -- not all scope implementations can handle them */
@Override
public void recordBeginBlock(IRClosure beginBlockClosure) {
if (beginBlocks == null) beginBlocks = new ArrayList<IRClosure>();
if (beginBlocks == null) beginBlocks = new ArrayList<>();
beginBlockClosure.setBeginEndBlock();
beginBlocks.add(beginBlockClosure);
}
Expand Down
6 changes: 2 additions & 4 deletions core/src/main/java/org/jruby/ir/interpreter/BeginEndInterpreterContext.java
View file
@@ -1,11 +1,9 @@
package org.jruby.ir.interpreter;

import java.util.ArrayList;
import java.util.List;
import org.jruby.ir.IRClosure;
import org.jruby.ir.IRScope;
import org.jruby.ir.instructions.Instr;
import org.jruby.ir.operands.WrappedIRClosure;

/**
* Script body and Evals both have begin/end bodies and need the same state
Expand All @@ -14,8 +12,8 @@
public class BeginEndInterpreterContext extends InterpreterContext {
private List<IRClosure> beginBlocks;

public BeginEndInterpreterContext(IRScope scope, Instr[] instructions, boolean rebuild) {
super(scope, instructions, rebuild);
public BeginEndInterpreterContext(IRScope scope, List<Instr> instructions) {
super(scope, instructions);

beginBlocks = scope.getBeginBlocks();
}
Expand Down
5 changes: 3 additions & 2 deletions core/src/main/java/org/jruby/ir/interpreter/ClosureInterpreterContext.java
View file
@@ -1,5 +1,6 @@
package org.jruby.ir.interpreter;

import java.util.List;
import org.jruby.ir.IRClosure;
import org.jruby.ir.instructions.Instr;
import org.jruby.runtime.DynamicScope;
Expand All @@ -9,8 +10,8 @@
* Interpreter knowledge needed to interpret a closure.
*/
public class ClosureInterpreterContext extends InterpreterContext {
public ClosureInterpreterContext(IRClosure scope, Instr[] instructions, boolean rebuild) {
super(scope, instructions, rebuild);
public ClosureInterpreterContext(IRClosure scope, List<Instr> instructions) {
super(scope, instructions);
}

/**
Expand Down
22 changes: 18 additions & 4 deletions core/src/main/java/org/jruby/ir/interpreter/InterpreterContext.java
View file
@@ -1,11 +1,13 @@
package org.jruby.ir.interpreter;

import java.util.List;
import org.jruby.ir.IRFlags;
import org.jruby.ir.IRMetaClassBody;
import org.jruby.ir.IRMethod;
import org.jruby.ir.IRModuleBody;
import org.jruby.ir.IRScope;
import org.jruby.ir.instructions.Instr;
import org.jruby.ir.instructions.LabelInstr;
import org.jruby.ir.representations.CFG;
import org.jruby.parser.StaticScope;
import org.jruby.runtime.DynamicScope;
Expand Down Expand Up @@ -43,10 +45,9 @@ public class InterpreterContext {
private int runCount = 0;
private boolean rebuilt = false;

public InterpreterContext(IRScope scope, Instr[] instructions, boolean rebuild) {
public InterpreterContext(IRScope scope, List<Instr> instructions) {
//FIXME: Remove once we conditionally plug in CFG on debug-only
this.cfg = scope.getCFG();
this.rebuilt = rebuild;
if (this.rebuilt) {
this.runCount = 30;
}
Expand All @@ -63,7 +64,7 @@ public InterpreterContext(IRScope scope, Instr[] instructions, boolean rebuild)
this.temporaryBooleanVariablecount = scope.getBooleanVariablesCount();
this.temporaryFixnumVariablecount = scope.getFixnumVariablesCount();
this.temporaryFloatVariablecount = scope.getFloatVariablesCount();
this.instructions = instructions;
this.instructions = prepareBuildInstructions(instructions);
this.hasExplicitCallProtocol = scope.getFlags().contains(IRFlags.HAS_EXPLICIT_CALL_PROTOCOL);
this.reuseParentDynScope = scope.getFlags().contains(IRFlags.REUSE_PARENT_DYNSCOPE);
this.pushNewDynScope = !scope.getFlags().contains(IRFlags.DYNSCOPE_ELIMINATED) && !reuseParentDynScope;
Expand All @@ -81,6 +82,19 @@ private InterpreterEngine determineInterpreterEngine(IRScope scope) {
}
}

private Instr[] prepareBuildInstructions(List<Instr> instructions) {
int length = instructions.size();
Instr[] linearizedInstrArray = instructions.toArray(new Instr[length]);
for (int ipc = 0; ipc < length; ipc++) {
Instr i = linearizedInstrArray[ipc];
i.setIPC(ipc);

if (i instanceof LabelInstr) ((LabelInstr) i).getLabel().setTargetPC(ipc + 1);
}

return linearizedInstrArray;
}

public CFG getCFG() {
return cfg;
}
Expand All @@ -90,7 +104,7 @@ public boolean isRebuilt() {
}

public void incrementRunCount() {
runCount++;
//runCount++;
}

public boolean needsRebuilding() {
Expand Down

2 comments on commit dee6c77

@cheald
Copy link
Contributor

@cheald cheald commented on dee6c77 Apr 2, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something in this commit broke something WRT requiring, according to bisect.

If you install ActiveSupport 4.2.1 (4.2.0 doesn't break), then bin/jruby -e "require 'active_support/all'", it results in:

$ bin/jruby -e "require 'active_support/all'"
/usr/local/rvm/gems/jruby-head/gems/activesupport-4.2.1/lib/active_support/values/time_zone.rb:37: warning: already initialized constant MAPPING
/usr/local/rvm/gems/jruby-head/gems/activesupport-4.2.1/lib/active_support/values/time_zone.rb:189: warning: already initialized constant UTC_OFFSET_WITH_COLON
/usr/local/rvm/gems/jruby-head/gems/activesupport-4.2.1/lib/active_support/values/time_zone.rb:190: warning: already initialized constant UTC_OFFSET_WITHOUT_COLON
NameError: uninitialized constant JSON::Ext::Parser
  const_missing at org/jruby/RubyModule.java:2938
         (root) at /usr/local/rvm/gems/jruby-head/gems/json-1.8.2-java/lib/json/ext.rb:16
        require at org/jruby/RubyKernel.java:966
         (root) at /var/www/repos/jruby/lib/ruby/stdlib/rubygems/core_ext/kernel_require.rb:1
        require at /var/www/repos/jruby/lib/ruby/stdlib/rubygems/core_ext/kernel_require.rb:69
        require at org/jruby/RubyKernel.java:966
         (root) at /usr/local/rvm/gems/jruby-head/gems/json-1.8.2-java/lib/json.rb:58
         (root) at /var/www/repos/jruby/lib/ruby/stdlib/rubygems/core_ext/kernel_require.rb:1
        require at org/jruby/RubyKernel.java:966
        require at /var/www/repos/jruby/lib/ruby/stdlib/rubygems/core_ext/kernel_require.rb:69
         (root) at /usr/local/rvm/gems/jruby-head/gems/activesupport-4.2.1/lib/active_support/core_ext/object/json.rb:2
        require at org/jruby/RubyKernel.java:966
         (root) at /var/www/repos/jruby/lib/ruby/stdlib/rubygems/core_ext/kernel_require.rb:1
        require at /var/www/repos/jruby/lib/ruby/stdlib/rubygems/core_ext/kernel_require.rb:69
           each at org/jruby/RubyArray.java:1569
         (root) at /usr/local/rvm/gems/jruby-head/gems/activesupport-4.2.1/lib/active_support/core_ext/object.rb:12
        require at org/jruby/RubyKernel.java:966
         (root) at /var/www/repos/jruby/lib/ruby/stdlib/rubygems/core_ext/kernel_require.rb:1
        require at /var/www/repos/jruby/lib/ruby/stdlib/rubygems/core_ext/kernel_require.rb:121
        require at org/jruby/RubyKernel.java:966
         (root) at /usr/local/rvm/gems/jruby-head/gems/activesupport-4.2.1/lib/active_support/core_ext.rb:2
     __script__ at -e:1

As of the previous commit, this works just fine.

@cheald
Copy link
Contributor

@cheald cheald commented on dee6c77 Apr 2, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only significant change I can find here is that prepareInstructions() is no longer called anywhere as of this commit.

Please sign in to comment.