Skip to content

Commit

Permalink
Clone all instrs in linearized instr list so that when JIT starts it …
Browse files Browse the repository at this point in the history
…cannot mangle anything in those instrs. Cannot see any measurable startup impact in this change
  • Loading branch information
enebo committed Oct 10, 2014
1 parent 583bc54 commit 7a8bf5f
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 10 deletions.
28 changes: 18 additions & 10 deletions core/src/main/java/org/jruby/ir/IRScope.java
Expand Up @@ -475,16 +475,17 @@ private synchronized Instr[] prepareInstructions() {

setupLinearization();

SimpleCloneInfo cloneInfo = new SimpleCloneInfo(this, false);

This comment has been minimized.

Copy link
@subbuss

subbuss Oct 11, 2014

Contributor

You actually forgot to clone the instr and set it in the array ... I have a fix in my local branch .. and it is a great test for uncovering bugs in the cloning code :) ... gem list fails, and 20+ rubyspecs fails ... also discovered perf. benefit from specialized call instrs accidentally when I cloned after specializing and got the unspecialized versions ... 50% slower without specialized calls ... figured out what was going on after puzzling a bit. Anyway, just a heads up.

This comment has been minimized.

Copy link
@enebo

enebo Oct 13, 2014

Author Member

This is supremely strange. I did have newInstr(instr.clone(cloneInfo)) in there but somehow had removed it before committing. Perhaps I did it to test timing and forgot to add it back.

I have no doubt call specialization helps which to me means unboxing into method would also see an improvement (even though past experiments have not shown it).

// FIXME: If CFG (or linearizedBBList) knew number of instrs we could end up allocing better

// Pass 1. Set up IPCs for labels and instruct and build linear instr list
List<Instr> newInstrs = new ArrayList<Instr>();
HashMap<Label, Integer> labelIPCMap = new HashMap<Label, Integer>();
int ipc = 0;
for (BasicBlock b: linearizedBBList) {
Label l = b.getLabel();
labelIPCMap.put(l, ipc);
// This assumes if multiple equal/same labels exist which are scattered around the scope
// must be the same Java instance or only this one will get a targetPC set.
l.setTargetPC(ipc);
// All same-named labels must be same Java instance for this to work or we would need
// to examine all Label operands and update this as well which would be expensive.
b.getLabel().setTargetPC(ipc);
List<Instr> bbInstrs = b.getInstrs();
int bbInstrsLength = bbInstrs.size();
for (int i = 0; i < bbInstrsLength; i++) {
Expand All @@ -508,16 +509,23 @@ private synchronized Instr[] prepareInstructions() {
// System.out.println("SCOPE: " + getName());
// System.out.println("INSTRS: " + cfg().toStringInstrs());

// Pass 2: Use ipc info from before to mark all instrs rpc
linearizedInstrArray = newInstrs.toArray(new Instr[newInstrs.size()]);

// Pass 2: Use ipc info from previous to mark all linearized instrs rpc
ipc = 0;
for (BasicBlock b : linearizedBBList) {
BasicBlock rescuerBB = cfg().getRescuerBBFor(b);
int rescuerPC = (rescuerBB == null) ? -1 : rescuerBB.getLabel().getTargetPC();
for (Instr i : b.getInstrs()) {
i.setRPC(rescuerPC);
for (Instr instr : b.getInstrs()) {
// FIXME: If we did not omit instrs from previous pass we could end up just doing a
// a size and for loop this n times instead of walking an examining each instr
if (!(instr instanceof ReceiveSelfInstr)) {
linearizedInstrArray[ipc].setRPC(rescuerPC);
ipc++;
}
}
}

linearizedInstrArray = newInstrs.toArray(new Instr[newInstrs.size()]);
return linearizedInstrArray;
}

Expand Down
4 changes: 4 additions & 0 deletions core/src/main/java/org/jruby/ir/instructions/Instr.java
Expand Up @@ -58,12 +58,16 @@ public Operation getOperation() {
return operation;
}

@Interp
public int getIPC() { return ipc; }

@Interp
public void setIPC(int ipc) { this.ipc = ipc; }

@Interp
public int getRPC() { return rpc; }

@Interp
public void setRPC(int rpc) { this.rpc = rpc; }

// Does this instruction have side effects as a result of its operation
Expand Down
@@ -1,11 +1,16 @@
package org.jruby.ir.instructions;

import org.jruby.ir.IRVisitor;
import org.jruby.ir.Interp;
import org.jruby.ir.Operation;
import org.jruby.ir.operands.Operand;
import org.jruby.ir.operands.Variable;
import org.jruby.ir.transformations.inlining.CloneInfo;
import org.jruby.ir.transformations.inlining.SimpleCloneInfo;
import org.jruby.parser.StaticScope;
import org.jruby.runtime.DynamicScope;
import org.jruby.runtime.ThreadContext;
import org.jruby.runtime.builtin.IRubyObject;

public class ReceiveSelfInstr extends Instr implements ResultInstr, FixedArityInstr {
private Variable result;
Expand Down

0 comments on commit 7a8bf5f

Please sign in to comment.