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: b799bfff8df5
Choose a base ref
...
head repository: jruby/jruby
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: e9d5c9489262
Choose a head ref
  • 3 commits
  • 6 files changed
  • 1 contributor

Commits on Oct 5, 2015

  1. Enum#values is a new array every time, so avoid it for Visibility.

    Several places used Visibility.values to get a list of all
    Visibilities, with the result that they created a new array every
    time. There may be other enums with the same usage but they have
    not shown up in my profiles.
    headius committed Oct 5, 2015
    Copy the full SHA
    b33d760 View commit details
  2. Copy the full SHA
    52ab58d View commit details
  3. Share evalType threadlocal between mixed mode and compiled block.

    Fixes #3368
    
    evalType gets tweaked at various stages of block execution, and
    when there were two copies in MixedModeIRBlockBody and its
    contained CompiledIRBlockBody, they weren't being set consistently
    in both places. This change makes them share the threadlocal and
    fixes the issues in #3368 leading to eval not scoping right.
    
    The root problem was that evalType is used to indicate that a
    given body is being used for an eval, which in this case meant it
    was being used for a Class.new class eval. Without the flag, the
    search for a method def scope would walk past the Class.new block
    and out into the containing class, defining methods in the wrong
    place.
    
    TODO: Clean up state like evalType and do a better job of unifying
    the different container objects for block bodies.
    headius committed Oct 5, 2015
    Copy the full SHA
    e9d5c94 View commit details
Original file line number Diff line number Diff line change
@@ -239,7 +239,7 @@ public IRubyObject call(ThreadContext context, IRubyObject self, RubyModule klaz
* @return true if the call would not violate visibility; false otherwise
*/
public boolean isCallableFrom(IRubyObject caller, CallType callType) {
switch (Visibility.values()[visibility]) {
switch (Visibility.getValues()[visibility]) {
case PUBLIC:
return true;
case PRIVATE:
@@ -322,7 +322,7 @@ public void setImplementationClass(RubyModule implClass) {
* @return The visibility of this method
*/
public Visibility getVisibility() {
return Visibility.values()[visibility];
return Visibility.getValues()[visibility];
}

/**
Original file line number Diff line number Diff line change
@@ -2,9 +2,9 @@

import org.jruby.ir.IRVisitor;
import org.jruby.ir.Operation;
import org.jruby.ir.operands.Operand;
import org.jruby.ir.operands.Variable;
import org.jruby.ir.persistence.IRReaderDecoder;
import org.jruby.ir.runtime.IRRuntimeHelpers;
import org.jruby.ir.transformations.inlining.CloneInfo;
import org.jruby.parser.StaticScope;
import org.jruby.runtime.DynamicScope;
@@ -28,12 +28,7 @@ public void visit(IRVisitor visitor) {

@Override
public Object interpret(ThreadContext context, StaticScope currScope, DynamicScope currDynScope, IRubyObject self, Object[] temp) {
int i = 0;
while (!currDynScope.getStaticScope().isArgumentScope()) {
currDynScope = currDynScope.getParentScope();
i++;
}
return context.runtime.newFixnum(i);
return IRRuntimeHelpers.getArgScopeDepth(context, currScope);
}

public static ArgScopeDepthInstr decode(IRReaderDecoder d) {
10 changes: 10 additions & 0 deletions core/src/main/java/org/jruby/ir/runtime/IRRuntimeHelpers.java
Original file line number Diff line number Diff line change
@@ -1442,4 +1442,14 @@ public static IRubyObject getVariableWithAccessor(IRubyObject self, VariableAcce
public static void setVariableWithAccessor(IRubyObject self, IRubyObject value, VariableAccessor accessor) {
accessor.set(self, value);
}

@JIT
public static RubyFixnum getArgScopeDepth(ThreadContext context, StaticScope currScope) {
int i = 0;
while (!currScope.isArgumentScope()) {
currScope = currScope.getEnclosingScope();
i++;
}
return context.runtime.newFixnum(i);
}
}
12 changes: 9 additions & 3 deletions core/src/main/java/org/jruby/ir/targets/JVMVisitor.java
Original file line number Diff line number Diff line change
@@ -415,6 +415,14 @@ public void AliasInstr(AliasInstr aliasInstr) {
m.invokeIRHelper("defineAlias", sig(void.class, ThreadContext.class, IRubyObject.class, DynamicScope.class, String.class, String.class));
}

@Override
public void ArgScopeDepthInstr(ArgScopeDepthInstr instr) {
jvmMethod().loadContext();
jvmMethod().loadStaticScope();
jvmMethod().invokeIRHelper("getArgScopeDepth", sig(RubyFixnum.class, ThreadContext.class, StaticScope.class));
jvmStoreLocal(instr.getResult());
}

@Override
public void AttrAssignInstr(AttrAssignInstr attrAssignInstr) {
Operand[] callArgs = attrAssignInstr.getCallArgs();
@@ -1425,9 +1433,7 @@ public void PushFrameInstr(PushFrameInstr pushframeinstr) {
// FIXME: this should be part of explicit call protocol only when needed, optimizable, and correct for the scope
// See also CompiledIRMethod.call
jvmMethod().loadContext();
jvmAdapter().invokestatic(p(Visibility.class), "values", sig(Visibility[].class));
jvmAdapter().ldc(Visibility.PUBLIC.ordinal());
jvmAdapter().aaload();
jvmAdapter().getstatic(p(Visibility.class), "PUBLIC", ci(Visibility.class));
jvmAdapter().invokevirtual(p(ThreadContext.class), "setCurrentVisibility", sig(void.class, Visibility.class));
}

Original file line number Diff line number Diff line change
@@ -49,6 +49,7 @@ public void setCallCount(int callCount) {
@Override
public void completeBuild(CompiledIRBlockBody blockBody) {
this.callCount = -1;
blockBody.evalType = this.evalType; // share with parent
this.jittedBody = blockBody;
}

6 changes: 6 additions & 0 deletions core/src/main/java/org/jruby/runtime/Visibility.java
Original file line number Diff line number Diff line change
@@ -27,6 +27,8 @@

public enum Visibility {
PUBLIC, PROTECTED, PRIVATE, MODULE_FUNCTION;

private static final Visibility[] VALUES = values();

public boolean isPrivate() {
return this == PRIVATE;
@@ -35,4 +37,8 @@ public boolean isPrivate() {
public boolean isProtected() {
return this == PROTECTED;
}

public static Visibility[] getValues() {
return VALUES;
}
}