From 8e6506821755267d4c31de4104877e926727c28b Mon Sep 17 00:00:00 2001 From: "Thomas E. Enebo" Date: Fri, 18 Aug 2023 15:01:41 -0400 Subject: [PATCH 1/4] Optimize both @a ||= 1 and defined?(@a). Changes: 1. @a ||= 1 will not use defined? at all but use get_field. get_field uses a cached VariableAccessor so repeated calls will get the value much faster. Secondly by not using defined? it is able to go from 2 branches to 1 branch. This reduces resulting codesize a lot. 2. defined?(@a) is now written in terms of get_field. get_field itself added an option rawValue which will return %undefined instead of nil when @a is not present. This allows us to rewrite defined?(@) in terms of an if/else branch (in IR) instead of the baked in ruby runtime helper instr. The general notion behind both is to use instrs which has a variable accessor cache. An interesting wrinkle is that indy on failed defined? is only a tiny bit faster than it was. I think this because the cache only fails and the new version of defined?(@a) uses IR for branching. On the other hand if @a does exist we get an amazing perf boost. --- .../src/main/java/org/jruby/ir/IRBuilder.java | 42 +++++++++++++------ .../jruby/ir/instructions/GetFieldInstr.java | 33 +++++++++++---- .../ExitableInterpreterEngine.java | 11 ----- .../interpreter/StartupInterpreterEngine.java | 11 ----- .../ir/targets/InstanceVariableCompiler.java | 3 +- .../java/org/jruby/ir/targets/JVMVisitor.java | 2 +- .../indy/IndyInstanceVariableCompiler.java | 4 +- .../NormalInstanceVariableCompiler.java | 10 +++-- .../runtime/invokedynamic/VariableSite.java | 11 +++-- .../jruby/runtime/ivars/VariableAccessor.java | 11 +++++ 10 files changed, 86 insertions(+), 52 deletions(-) diff --git a/core/src/main/java/org/jruby/ir/IRBuilder.java b/core/src/main/java/org/jruby/ir/IRBuilder.java index ec8bdc3abb4..bd21410d5a3 100644 --- a/core/src/main/java/org/jruby/ir/IRBuilder.java +++ b/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(); diff --git a/core/src/main/java/org/jruby/ir/instructions/GetFieldInstr.java b/core/src/main/java/org/jruby/ir/instructions/GetFieldInstr.java index b890c145f16..cceb7fc0c85 100644 --- a/core/src/main/java/org/jruby/ir/instructions/GetFieldInstr.java +++ b/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}; + } } diff --git a/core/src/main/java/org/jruby/ir/interpreter/ExitableInterpreterEngine.java b/core/src/main/java/org/jruby/ir/interpreter/ExitableInterpreterEngine.java index 298508643a0..a45c55e64c5 100644 --- a/core/src/main/java/org/jruby/ir/interpreter/ExitableInterpreterEngine.java +++ b/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(), diff --git a/core/src/main/java/org/jruby/ir/interpreter/StartupInterpreterEngine.java b/core/src/main/java/org/jruby/ir/interpreter/StartupInterpreterEngine.java index aa2732ea5a0..19953081d08 100644 --- a/core/src/main/java/org/jruby/ir/interpreter/StartupInterpreterEngine.java +++ b/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(), diff --git a/core/src/main/java/org/jruby/ir/targets/InstanceVariableCompiler.java b/core/src/main/java/org/jruby/ir/targets/InstanceVariableCompiler.java index dcd1e5595c9..6c886d9a123 100644 --- a/core/src/main/java/org/jruby/ir/targets/InstanceVariableCompiler.java +++ b/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); } diff --git a/core/src/main/java/org/jruby/ir/targets/JVMVisitor.java b/core/src/main/java/org/jruby/ir/targets/JVMVisitor.java index ce2bef5fe93..6aaefb860f6 100644 --- a/core/src/main/java/org/jruby/ir/targets/JVMVisitor.java +++ b/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()); } diff --git a/core/src/main/java/org/jruby/ir/targets/indy/IndyInstanceVariableCompiler.java b/core/src/main/java/org/jruby/ir/targets/indy/IndyInstanceVariableCompiler.java index 441d83c40e5..3b7d57df381 100644 --- a/core/src/main/java/org/jruby/ir/targets/indy/IndyInstanceVariableCompiler.java +++ b/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); } } diff --git a/core/src/main/java/org/jruby/ir/targets/simple/NormalInstanceVariableCompiler.java b/core/src/main/java/org/jruby/ir/targets/simple/NormalInstanceVariableCompiler.java index 3c1ae5d354d..48f655f87e9 100644 --- a/core/src/main/java/org/jruby/ir/targets/simple/NormalInstanceVariableCompiler.java +++ b/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)); + } } /** diff --git a/core/src/main/java/org/jruby/runtime/invokedynamic/VariableSite.java b/core/src/main/java/org/jruby/runtime/invokedynamic/VariableSite.java index acc0b2a1b51..232d6071e3e 100644 --- a/core/src/main/java/org/jruby/runtime/invokedynamic/VariableSite.java +++ b/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); @@ -156,7 +161,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 { diff --git a/core/src/main/java/org/jruby/runtime/ivars/VariableAccessor.java b/core/src/main/java/org/jruby/runtime/ivars/VariableAccessor.java index ca35a14deda..66e6a308c72 100644 --- a/core/src/main/java/org/jruby/runtime/ivars/VariableAccessor.java +++ b/core/src/main/java/org/jruby/runtime/ivars/VariableAccessor.java @@ -27,8 +27,10 @@ package org.jruby.runtime.ivars; +import org.jruby.Ruby; import org.jruby.RubyBasicObject; import org.jruby.RubyClass; +import org.jruby.ir.operands.UndefinedValue; import org.jruby.runtime.ThreadContext; import org.jruby.runtime.builtin.IRubyObject; @@ -100,6 +102,15 @@ public Object get(Object object) { return getVariable((RubyBasicObject)object, index); } + /** + * Retrieve object or IR %undefined + */ + public Object getOrUndefined(Object object) { + Object value = getVariable((RubyBasicObject) object, index); + + return value == null ? UndefinedValue.UNDEFINED : value; + } + /** * Retrieve the variable's value from the given object. * From 1a4d083cfa864b01c515f656beb24aef55921e73 Mon Sep 17 00:00:00 2001 From: "Thomas E. Enebo" Date: Fri, 18 Aug 2023 16:21:00 -0400 Subject: [PATCH 2/4] field variable accessor missed in variable changes --- .../jruby/ir/instructions/RuntimeHelperCall.java | 8 +------- .../org/jruby/ir/runtime/IRRuntimeHelpers.java | 6 ------ .../java/org/jruby/ir/targets/JVMVisitor.java | 8 -------- .../runtime/ivars/FieldVariableAccessor.java | 16 ++++++++++++++++ 4 files changed, 17 insertions(+), 21 deletions(-) diff --git a/core/src/main/java/org/jruby/ir/instructions/RuntimeHelperCall.java b/core/src/main/java/org/jruby/ir/instructions/RuntimeHelperCall.java index abb35ebd4f3..d1ce6d1cd97 100644 --- a/core/src/main/java/org/jruby/ir/instructions/RuntimeHelperCall.java +++ b/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, diff --git a/core/src/main/java/org/jruby/ir/runtime/IRRuntimeHelpers.java b/core/src/main/java/org/jruby/ir/runtime/IRRuntimeHelpers.java index 029428556d0..8073b8aa9a4 100644 --- a/core/src/main/java/org/jruby/ir/runtime/IRRuntimeHelpers.java +++ b/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); diff --git a/core/src/main/java/org/jruby/ir/targets/JVMVisitor.java b/core/src/main/java/org/jruby/ir/targets/JVMVisitor.java index 6aaefb860f6..e5ff0d0fb8d 100644 --- a/core/src/main/java/org/jruby/ir/targets/JVMVisitor.java +++ b/core/src/main/java/org/jruby/ir/targets/JVMVisitor.java @@ -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]); diff --git a/core/src/main/java/org/jruby/runtime/ivars/FieldVariableAccessor.java b/core/src/main/java/org/jruby/runtime/ivars/FieldVariableAccessor.java index 0cbe2a137ad..1de1b6a1f11 100644 --- a/core/src/main/java/org/jruby/runtime/ivars/FieldVariableAccessor.java +++ b/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; @@ -91,6 +92,21 @@ public Object get(Object object) { } } + /** + * Retrieve the variable's value from the given object. + * + * @param object the object from which to retrieve this variable + * @return the variable's value or %undefined + */ + public Object getOrUndefined(Object object) { + try { + Object value = getter.invoke(object); + return value == null ? UndefinedValue.UNDEFINED : (IRubyObject) value; + } catch (Throwable t) { + Helpers.throwException(t); + return null; + } + } /** * Retrieve the variable's value from the given object. * From 1fa1e5f979aac954ff9bbba55b81a4a91023102a Mon Sep 17 00:00:00 2001 From: "Thomas E. Enebo" Date: Fri, 18 Aug 2023 16:43:36 -0400 Subject: [PATCH 3/4] Generalize this by calling get in base class vs override all types --- .../runtime/ivars/FieldVariableAccessor.java | 15 --------------- .../org/jruby/runtime/ivars/VariableAccessor.java | 3 +-- 2 files changed, 1 insertion(+), 17 deletions(-) diff --git a/core/src/main/java/org/jruby/runtime/ivars/FieldVariableAccessor.java b/core/src/main/java/org/jruby/runtime/ivars/FieldVariableAccessor.java index 1de1b6a1f11..4136b75f605 100644 --- a/core/src/main/java/org/jruby/runtime/ivars/FieldVariableAccessor.java +++ b/core/src/main/java/org/jruby/runtime/ivars/FieldVariableAccessor.java @@ -92,21 +92,6 @@ public Object get(Object object) { } } - /** - * Retrieve the variable's value from the given object. - * - * @param object the object from which to retrieve this variable - * @return the variable's value or %undefined - */ - public Object getOrUndefined(Object object) { - try { - Object value = getter.invoke(object); - return value == null ? UndefinedValue.UNDEFINED : (IRubyObject) value; - } catch (Throwable t) { - Helpers.throwException(t); - return null; - } - } /** * Retrieve the variable's value from the given object. * diff --git a/core/src/main/java/org/jruby/runtime/ivars/VariableAccessor.java b/core/src/main/java/org/jruby/runtime/ivars/VariableAccessor.java index 66e6a308c72..06095ef77fa 100644 --- a/core/src/main/java/org/jruby/runtime/ivars/VariableAccessor.java +++ b/core/src/main/java/org/jruby/runtime/ivars/VariableAccessor.java @@ -106,8 +106,7 @@ public Object get(Object object) { * Retrieve object or IR %undefined */ public Object getOrUndefined(Object object) { - Object value = getVariable((RubyBasicObject) object, index); - + Object value = get(object); return value == null ? UndefinedValue.UNDEFINED : value; } From 69ffd8f25828e06212e3abca845b2b7c287611f6 Mon Sep 17 00:00:00 2001 From: "Thomas E. Enebo" Date: Sat, 19 Aug 2023 12:56:08 -0400 Subject: [PATCH 4/4] indy fix --- core/src/main/java/org/jruby/Ruby.java | 12 ++++++++++++ core/src/main/java/org/jruby/runtime/Helpers.java | 4 ++++ .../jruby/runtime/invokedynamic/VariableSite.java | 4 +++- 3 files changed, 19 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/org/jruby/Ruby.java b/core/src/main/java/org/jruby/Ruby.java index b298297f872..2a6d5214e93 100644 --- a/core/src/main/java/org/jruby/Ruby.java +++ b/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 POPULATORS = new ClassValue() { @Override diff --git a/core/src/main/java/org/jruby/runtime/Helpers.java b/core/src/main/java/org/jruby/runtime/Helpers.java index 677ee25e2e6..7786cbd7865 100644 --- a/core/src/main/java/org/jruby/runtime/Helpers.java +++ b/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) { diff --git a/core/src/main/java/org/jruby/runtime/invokedynamic/VariableSite.java b/core/src/main/java/org/jruby/runtime/invokedynamic/VariableSite.java index 232d6071e3e..4d2b188b1d3 100644 --- a/core/src/main/java/org/jruby/runtime/invokedynamic/VariableSite.java +++ b/core/src/main/java/org/jruby/runtime/invokedynamic/VariableSite.java @@ -101,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;