Merge pull request jruby#7896 from enebo/opor

Optimize @A ||= x and defined?(@A)

core/src/main/java/org/jruby/Ruby.java

@@ -4939,6 +4939,17 @@ public MethodHandle getNullToNilHandle() {
         return this.nullToNil = nullToNil;
     }
 
+    public MethodHandle getNullToUndefinedHandle() {
+        MethodHandle filter = this.nullToUndefined;
+
+        if (filter != null) return filter;
+
+        filter = InvokeDynamicSupport.findStatic(Helpers.class, "nullToUndefined", methodType(IRubyObject.class, IRubyObject.class));
+        filter = explicitCastArguments(filter, methodType(IRubyObject.class, Object.class));
+
+        return this.nullToUndefined = filter;
+    }
+
     // Parser stats methods
     private void addLoadParseToStats() {
         if (parserStats != null) parserStats.addLoadParse();
@@ -5745,6 +5756,7 @@ public int applyAsInt(ThreadContext context) {
      * The nullToNil filter for this runtime.
      */
     private MethodHandle nullToNil;
+    private MethodHandle nullToUndefined;
 
     public final ClassValue<TypePopulator> POPULATORS = new ClassValue<TypePopulator>() {
         @Override

core/src/main/java/org/jruby/ir/IRBuilder.java

@@ -2527,17 +2527,7 @@ public Operand buildGetDefinition(Node node) {
             );
         }
         case INSTVARNODE:
-            return addResultInstr(
-                    new RuntimeHelperCall(
-                            createTemporaryVariable(),
-                            IS_DEFINED_INSTANCE_VAR,
-                            new Operand[] {
-                                    buildSelf(),
-                                    new FrozenString(((InstVarNode) node).getName()),
-                                    new FrozenString(DefinedMessage.INSTANCE_VARIABLE.getText())
-                            }
-                    )
-            );
+            return buildInstVarGetDefinition((InstVarNode) node);
         case CLASSVARNODE:
             return addResultInstr(
                     new RuntimeHelperCall(
@@ -3879,7 +3869,7 @@ public Operand buildInstAsgn(final InstAsgnNode instAsgnNode) {
     }
 
     public Operand buildInstVar(InstVarNode node) {
-        return addResultInstr(new GetFieldInstr(createTemporaryVariable(), buildSelf(), node.getName()));
+        return addResultInstr(new GetFieldInstr(createTemporaryVariable(), buildSelf(), node.getName(), false));
     }
 
     private InterpreterContext buildIterInner(RubySymbol methodName, IterNode iterNode) {
@@ -4229,6 +4219,32 @@ public Operand buildOpAsgnAnd(OpAsgnAndNode andNode) {
         return result;
     }
 
+    private Operand buildInstVarOpAsgnOrNode(OpAsgnOrNode node) {
+        Label done = getNewLabel();
+        Variable result = addResultInstr(new GetFieldInstr(temp(), buildSelf(), ((InstVarNode) node.getFirstNode()).getName(), false));
+        addInstr(createBranch(result, manager.getTrue(), done));
+        Operand value = build(node.getSecondNode()); // This is an AST node that sets x = y, so nothing special to do here.
+        copy(result, value);
+        addInstr(new LabelInstr(done));
+
+        return result;
+    }
+
+    private Operand buildInstVarGetDefinition(InstVarNode node) {
+        Variable result = temp();
+        Label done = getNewLabel();
+        Label undefined = getNewLabel();
+        Variable value = addResultInstr(new GetFieldInstr(temp(), buildSelf(), node.getName(), true));
+        addInstr(createBranch(value, UndefinedValue.UNDEFINED, undefined));
+        copy(result, new FrozenString(DefinedMessage.INSTANCE_VARIABLE.getText()));
+        jump(done);
+        addInstr(new LabelInstr(undefined));
+        copy(result, buildNil());
+        addInstr(new LabelInstr(done));
+
+        return result;
+    }
+
     // "x ||= y"
     // --> "x = (is_defined(x) && is_true(x) ? x : y)"
     // --> v = -- build(x) should return a variable! --
@@ -4238,6 +4254,8 @@ public Operand buildOpAsgnAnd(OpAsgnAndNode andNode) {
     //   L:
     //
     public Operand buildOpAsgnOr(final OpAsgnOrNode orNode) {
+        if (orNode.getFirstNode() instanceof InstVarNode) return buildInstVarOpAsgnOrNode(orNode);
+
         Label    l1 = getNewLabel();
         Label    l2 = null;
         Variable flag = createTemporaryVariable();

core/src/main/java/org/jruby/ir/instructions/GetFieldInstr.java

@@ -6,8 +6,10 @@
 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;
@@ -22,18 +24,29 @@
 public class GetFieldInstr extends GetInstr implements FixedArityInstr {
     private transient VariableAccessor accessor = VariableAccessor.DUMMY_ACCESSOR;
 
-    public GetFieldInstr(Variable dest, Operand obj, RubySymbol fieldName) {
+    // do not return as nil if it doesn't exist but as undefined.
+    public final boolean rawValue;
+
+    public GetFieldInstr(Variable dest, Operand obj, RubySymbol fieldName, boolean rawValue) {
         super(Operation.GET_FIELD, dest, obj, fieldName);
+
+        this.rawValue = rawValue;
     }
 
     @Override
     public Instr clone(CloneInfo ii) {
         return new GetFieldInstr(ii.getRenamedVariable(getResult()),
-                getSource().cloneForInlining(ii), getName());
+                getSource().cloneForInlining(ii), getName(), rawValue);
     }
 
     public static GetFieldInstr decode(IRReaderDecoder d) {
-        return new GetFieldInstr(d.decodeVariable(), d.decodeOperand(), d.decodeSymbol());
+        return new GetFieldInstr(d.decodeVariable(), d.decodeOperand(), d.decodeSymbol(), d.decodeBoolean());
+    }
+
+    @Override
+    public void encode(IRWriterEncoder e) {
+        super.encode(e);
+        e.encode(rawValue);
     }
 
     public VariableAccessor getAccessor(IRubyObject o) {
@@ -51,15 +64,19 @@ public VariableAccessor getAccessor(IRubyObject o) {
     public Object interpret(ThreadContext context, StaticScope currScope, DynamicScope currDynScope, IRubyObject self, Object[] temp) {
         IRubyObject object = (IRubyObject) getSource().retrieve(context, self, currScope, currDynScope, temp);
         VariableAccessor a = getAccessor(object);
-        Object result = a == null ? null : (IRubyObject)a.get(object);
-        if (result == null) {
-            result = context.nil;
-        }
-        return result;
+        Object result = a == null ? null : (IRubyObject) a.get(object);
+
+        if (result == null && rawValue) return UndefinedValue.UNDEFINED;
+        return result == null && !rawValue ? context.nil : result;
     }
 
     @Override
     public void visit(IRVisitor visitor) {
         visitor.GetFieldInstr(this);
     }
+
+    @Override
+    public String[] toStringNonOperandArgs() {
+        return new String[] {"name: " + getName(), "raw: " + rawValue};
+    }
 }

core/src/main/java/org/jruby/ir/instructions/RuntimeHelperCall.java

@@ -25,7 +25,7 @@
 public class RuntimeHelperCall extends NOperandResultBaseInstr {
     public enum Methods {
         HANDLE_PROPAGATED_BREAK, HANDLE_NONLOCAL_RETURN, HANDLE_BREAK_AND_RETURNS_IN_LAMBDA,
-        IS_DEFINED_BACKREF, IS_DEFINED_NTH_REF, IS_DEFINED_GLOBAL, IS_DEFINED_INSTANCE_VAR,
+        IS_DEFINED_BACKREF, IS_DEFINED_NTH_REF, IS_DEFINED_GLOBAL,
         IS_DEFINED_CLASS_VAR, IS_DEFINED_SUPER, IS_DEFINED_METHOD, IS_DEFINED_CALL,
         IS_DEFINED_CONSTANT_OR_METHOD, MERGE_KWARGS, IS_HASH_EMPTY, HASH_CHECK, ARRAY_LENGTH;
 
@@ -138,12 +138,6 @@ public Object callHelper(ThreadContext context, StaticScope currScope, DynamicSc
                         ((FrozenString) operands[1]).retrieve(context, self, currScope, currDynScope, temp),
                         (IRubyObject) operands[2].retrieve(context, self, currScope, currDynScope, temp),
                         (IRubyObject) operands[3].retrieve(context, self, currScope, currDynScope, temp));
-            case IS_DEFINED_INSTANCE_VAR:
-                return IRRuntimeHelpers.isDefinedInstanceVar(
-                        context,
-                        (IRubyObject) arg1,
-                        ((Stringable) operands[1]).getString(),
-                        (IRubyObject) operands[2].retrieve(context, self, currScope, currDynScope, temp));
             case IS_DEFINED_CLASS_VAR:
                 return IRRuntimeHelpers.isDefinedClassVar(
                         context,

core/src/main/java/org/jruby/ir/interpreter/ExitableInterpreterEngine.java

@@ -178,17 +178,6 @@ protected static void processOtherOp(ThreadContext context, Block block, Instr i
                 setResult(temp, currDynScope, c.getResult(), retrieveOp(c.getSource(), context, self, currDynScope, currScope, temp));
                 break;
             }
-            case GET_FIELD: {
-                GetFieldInstr gfi = (GetFieldInstr)instr;
-                IRubyObject object = (IRubyObject)gfi.getSource().retrieve(context, self, currScope, currDynScope, temp);
-                VariableAccessor a = gfi.getAccessor(object);
-                Object result = a == null ? null : (IRubyObject)a.get(object);
-                if (result == null) {
-                    result = context.nil;
-                }
-                setResult(temp, currDynScope, gfi.getResult(), result);
-                break;
-            }
             case RUNTIME_HELPER: {
                 RuntimeHelperCall rhc = (RuntimeHelperCall)instr;
                 setResult(temp, currDynScope, rhc.getResult(),

core/src/main/java/org/jruby/ir/interpreter/StartupInterpreterEngine.java

@@ -132,17 +132,6 @@ protected static void processOtherOp(ThreadContext context, Block block, Instr i
                 setResult(temp, currDynScope, c.getResult(), retrieveOp(c.getSource(), context, self, currDynScope, currScope, temp));
                 break;
             }
-            case GET_FIELD: {
-                GetFieldInstr gfi = (GetFieldInstr)instr;
-                IRubyObject object = (IRubyObject)gfi.getSource().retrieve(context, self, currScope, currDynScope, temp);
-                VariableAccessor a = gfi.getAccessor(object);
-                Object result = a == null ? null : (IRubyObject)a.get(object);
-                if (result == null) {
-                    result = context.nil;
-                }
-                setResult(temp, currDynScope, gfi.getResult(), result);
-                break;
-            }
             case RUNTIME_HELPER: {
                 RuntimeHelperCall rhc = (RuntimeHelperCall)instr;
                 setResult(temp, currDynScope, rhc.getResult(),

core/src/main/java/org/jruby/ir/runtime/IRRuntimeHelpers.java

@@ -958,12 +958,6 @@ public static IRubyObject isDefinedClassVar(ThreadContext context, RubyModule re
         return defined ? definedMessage : context.nil;
     }
 
-    @JIT @Interp
-    public static IRubyObject isDefinedInstanceVar(ThreadContext context, IRubyObject receiver, String name, IRubyObject definedMessage) {
-        return receiver.getInstanceVariables().hasInstanceVariable(name) ?
-                definedMessage : context.nil;
-    }
-
     @JIT @Interp
     public static IRubyObject isDefinedCall(ThreadContext context, IRubyObject self, IRubyObject receiver, String name, IRubyObject definedMessage) {
         IRubyObject boundValue = Helpers.getDefinedCall(context, self, receiver, name, definedMessage);

core/src/main/java/org/jruby/ir/targets/InstanceVariableCompiler.java

@@ -21,6 +21,7 @@ public interface InstanceVariableCompiler {
      *
      * @param source runnable to push source object, may be called twice
      * @param name name of variable to load
+     * @param rawValue should the result be null instead of nil on non-existent field
      */
-    public abstract void getField(Runnable source, String name);
+    public abstract void getField(Runnable source, String name, boolean rawValue);
 }

core/src/main/java/org/jruby/ir/targets/JVMVisitor.java

@@ -1614,7 +1614,7 @@ public void GetFieldInstr(GetFieldInstr getfieldinstr) {
             throw new NotCompilableException("non-propagatable target for PutField: " + source);
         }
 
-        jvmMethod().getInstanceVariableCompiler().getField(() -> visit(source), getfieldinstr.getId());
+        jvmMethod().getInstanceVariableCompiler().getField(() -> visit(source), getfieldinstr.getId(), getfieldinstr.rawValue);
         jvmStoreLocal(getfieldinstr.getResult());
     }
 
@@ -2295,14 +2295,6 @@ public void RuntimeHelperCall(RuntimeHelperCall runtimehelpercall) {
                 jvmAdapter().invokestatic(p(IRRuntimeHelpers.class), "isDefinedGlobal", sig(IRubyObject.class, ThreadContext.class, String.class, IRubyObject.class));
                 jvmStoreLocal(runtimehelpercall.getResult());
                 break;
-            case IS_DEFINED_INSTANCE_VAR:
-                jvmMethod().loadContext();
-                visit(runtimehelpercall.getArgs()[0]);
-                jvmAdapter().ldc(((Stringable)runtimehelpercall.getArgs()[1]).getString());
-                visit(runtimehelpercall.getArgs()[2]);
-                jvmAdapter().invokestatic(p(IRRuntimeHelpers.class), "isDefinedInstanceVar", sig(IRubyObject.class, ThreadContext.class, IRubyObject.class, String.class, IRubyObject.class));
-                jvmStoreLocal(runtimehelpercall.getResult());
-                break;
             case IS_DEFINED_CLASS_VAR:
                 jvmMethod().loadContext();
                 visit(runtimehelpercall.getArgs()[0]);

core/src/main/java/org/jruby/ir/targets/indy/IndyInstanceVariableCompiler.java

@@ -23,8 +23,8 @@ public void putField(Runnable target, Runnable value, String name) {
         compiler.adapter.invokedynamic("ivarSet:" + JavaNameMangler.mangleMethodName(name), sig(void.class, IRubyObject.class, IRubyObject.class), VariableSite.IVAR_ASM_HANDLE);
     }
 
-    public void getField(Runnable source, String name) {
+    public void getField(Runnable source, String name, boolean rawValue) {
         source.run();
-        compiler.adapter.invokedynamic("ivarGet:" + JavaNameMangler.mangleMethodName(name), CodegenUtils.sig(JVM.OBJECT, IRubyObject.class), VariableSite.IVAR_ASM_HANDLE);
+        compiler.adapter.invokedynamic("ivarGet:" + JavaNameMangler.mangleMethodName(name) + (rawValue ? ":true" : ""), CodegenUtils.sig(JVM.OBJECT, IRubyObject.class), VariableSite.IVAR_ASM_HANDLE);
     }
 }

core/src/main/java/org/jruby/ir/targets/simple/NormalInstanceVariableCompiler.java

@@ -33,12 +33,16 @@ public void putField(Runnable target, Runnable value, String name) {
         compiler.adapter.invokevirtual(p(VariableAccessor.class), "set", sig(void.class, Object.class, Object.class));
     }
 
-    public void getField(Runnable source, String name) {
+    public void getField(Runnable source, String name, boolean rawValue) {
         source.run();
         cacheVariableAccessor(name, false);
         source.run();
-        compiler.loadContext();
-        compiler.adapter.invokevirtual(p(VariableAccessor.class), "getOrNil", sig(IRubyObject.class, Object.class, ThreadContext.class));
+        if (rawValue) {
+            compiler.adapter.invokevirtual(p(VariableAccessor.class), "getOrUndefined", sig(Object.class, Object.class));
+        } else {
+            compiler.loadContext();
+            compiler.adapter.invokevirtual(p(VariableAccessor.class), "getOrNil", sig(IRubyObject.class, Object.class, ThreadContext.class));
+        }
     }
 
     /**

core/src/main/java/org/jruby/runtime/Helpers.java

@@ -920,6 +920,10 @@ public static IRubyObject nullToNil(IRubyObject value, IRubyObject nil) {
         return value != null ? value : nil;
     }
 
+    public static IRubyObject nullToUndefined(IRubyObject value) {
+        return value != null ? value : UndefinedValue.UNDEFINED;
+    }
+
     public static void handleArgumentSizes(ThreadContext context, Ruby runtime, int given, int required, int opt, int rest) {
         if (opt == 0) {
             if (rest < 0) {

core/src/main/java/org/jruby/runtime/invokedynamic/VariableSite.java

@@ -2,6 +2,7 @@
 
 import org.jruby.RubyBasicObject;
 import org.jruby.RubyClass;
+import org.jruby.ir.operands.UndefinedValue;
 import org.jruby.runtime.builtin.IRubyObject;
 
 import java.lang.invoke.CallSite;
@@ -31,15 +32,18 @@
 public class VariableSite extends MutableCallSite {
     private final String name;
     private VariableAccessor accessor = VariableAccessor.DUMMY_ACCESSOR;
+
+    private boolean rawValue;
     private final String file;
     private final int line;
     private int chainCount;
 
     private static final Logger LOG = LoggerFactory.getLogger(VariableSite.class);
 
-    public VariableSite(MethodType type, String name, String file, int line) {
+    public VariableSite(MethodType type, String name, boolean rawValue, String file, int line) {
         super(type);
         this.name = name;
+        this.rawValue = rawValue;
         this.file = file;
         this.line = line;
         this.chainCount = 0;
@@ -80,7 +84,8 @@ public static CallSite ivar(MethodHandles.Lookup lookup, String name, MethodType
         String[] names = name.split(":");
         String operation = names[0];
         String varName = names[1];
-        VariableSite site = new VariableSite(type, varName, "noname", 0);
+        boolean rawValue = names.length > 2;
+        VariableSite site = new VariableSite(type, varName, rawValue, "noname", 0);
         MethodHandle handle;
 
         handle = lookup.findVirtual(VariableSite.class, operation, type);
@@ -96,7 +101,9 @@ public IRubyObject ivarGet(IRubyObject self) throws Throwable {
         VariableAccessor accessor = realClass.getVariableAccessorForRead(name());
 
         // produce nil if the variable has not been initialize
-        MethodHandle nullToNil = self.getRuntime().getNullToNilHandle();
+        MethodHandle nullToNil = rawValue ?
+                self.getRuntime().getNullToUndefinedHandle() :
+                self.getRuntime().getNullToNilHandle();
 
         // get variable value and filter with nullToNil
         MethodHandle getValue;
@@ -156,7 +163,7 @@ public IRubyObject ivarGetFail(IRubyObject self) {
         if (value != null) {
             return value;
         }
-        return self.getRuntime().getNil();
+        return rawValue ? UndefinedValue.UNDEFINED : self.getRuntime().getNil();
     }
 
     public void ivarSet(IRubyObject self, IRubyObject value) throws Throwable {

core/src/main/java/org/jruby/runtime/ivars/FieldVariableAccessor.java

@@ -30,6 +30,7 @@
 import com.headius.invokebinder.Binder;
 import org.jruby.RubyBasicObject;
 import org.jruby.RubyClass;
+import org.jruby.ir.operands.UndefinedValue;
 import org.jruby.runtime.Helpers;
 import org.jruby.runtime.ThreadContext;
 import org.jruby.runtime.builtin.IRubyObject;