Wire up invokedynamic caching for global variable reads.

This improves global read perf for #3350 but global variable writes still seem to be slower than MRI. I'm not exactly sure why but it probably has to do with doing a full lookup for our too- abstract wrapper around these values.
jruby · Sep 25, 2015 · f2612a2 · f2612a2
1 parent 6a875ce
commit f2612a2
Showing 8 changed files with 144 additions and 35 deletions.
diff --git a/core/src/main/java/org/jruby/internal/runtime/GlobalVariables.java b/core/src/main/java/org/jruby/internal/runtime/GlobalVariables.java
@@ -48,7 +48,7 @@
  */
 public class GlobalVariables {
     private Ruby runtime;
-    private Map<String, GlobalVariable> globalVariables = new ConcurrentHashMap<String, GlobalVariable>();
+    private final Map<String, GlobalVariable> globalVariables = new ConcurrentHashMap<String, GlobalVariable>();
 
     public GlobalVariables(Ruby runtime) {
         this.runtime = runtime;
@@ -116,7 +116,7 @@ public GlobalVariable getVariable(String name) {
 	    assert name != null;
 	    assert name.startsWith("$");
 
-	    GlobalVariable variable = (GlobalVariable)globalVariables.get(name);
+	    GlobalVariable variable = globalVariables.get(name);
         if (variable != null) return variable;
 
         return createIfNotDefined(name);
@@ -161,7 +161,7 @@ public void untraceVar(String name) {
         assert name.startsWith("$");
 
         if (isDefined(name)) {
-            GlobalVariable variable = (GlobalVariable)globalVariables.get(name);
+            GlobalVariable variable = globalVariables.get(name);
             variable.removeTraces();
         }
     }
@@ -171,7 +171,7 @@ public Set<String> getNames() {
     }
 
     private GlobalVariable createIfNotDefined(String name) {
-        GlobalVariable variable = (GlobalVariable)globalVariables.get(name);
+        GlobalVariable variable = globalVariables.get(name);
         if (variable == null) {
             variable = GlobalVariable.newUndefined(runtime, name);
             globalVariables.put(name, variable);

diff --git a/core/src/main/java/org/jruby/ir/instructions/GetGlobalVariableInstr.java b/core/src/main/java/org/jruby/ir/instructions/GetGlobalVariableInstr.java
@@ -5,7 +5,6 @@
 import org.jruby.ir.IRVisitor;
 import org.jruby.ir.Operation;
 import org.jruby.ir.operands.GlobalVariable;
-import org.jruby.ir.operands.Operand;
 import org.jruby.ir.operands.Variable;
 import org.jruby.ir.persistence.IRReaderDecoder;
 import org.jruby.ir.persistence.IRWriterEncoder;
@@ -24,12 +23,12 @@ public GetGlobalVariableInstr(Variable dest, GlobalVariable gvar) {
         super(Operation.GET_GLOBAL_VAR, dest, gvar);
     }
 
-    public GlobalVariable getGVar() {
+    public GlobalVariable getTarget() {
         return (GlobalVariable) getOperand1();
     }
 
     public boolean computeScopeFlags(IRScope scope) {
-        String name = getGVar().getName();
+        String name = getTarget().getName();
 
         if (name.equals("$_") || name.equals("$~") || name.equals("$`") || name.equals("$'") ||
             name.equals("$+") || name.equals("$LAST_READ_LINE") || name.equals("$LAST_MATCH_INFO") ||
@@ -43,13 +42,13 @@ public boolean computeScopeFlags(IRScope scope) {
 
     @Override
     public Instr clone(CloneInfo ii) {
-        return new GetGlobalVariableInstr(ii.getRenamedVariable(getResult()), getGVar().getName());
+        return new GetGlobalVariableInstr(ii.getRenamedVariable(getResult()), getTarget().getName());
     }
 
     @Override
     public void encode(IRWriterEncoder e) {
         super.encode(e);
-        e.encode(getGVar());
+        e.encode(getTarget());
     }
 
     public static GetGlobalVariableInstr decode(IRReaderDecoder d) {
@@ -58,7 +57,7 @@ public static GetGlobalVariableInstr decode(IRReaderDecoder d) {
 
     @Override
     public Object interpret(ThreadContext context, StaticScope currScope, DynamicScope currDynScope, IRubyObject self, Object[] temp) {
-        return getGVar().retrieve(context, self, currScope, currDynScope, temp);
+        return getTarget().retrieve(context, self, currScope, currDynScope, temp);
     }
 
     @Override

diff --git a/core/src/main/java/org/jruby/ir/targets/Bootstrap.java b/core/src/main/java/org/jruby/ir/targets/Bootstrap.java
@@ -8,6 +8,7 @@
 import org.jcodings.EncodingDB;
 import org.jruby.*;
 import org.jruby.common.IRubyWarnings;
+import org.jruby.internal.runtime.GlobalVariable;
 import org.jruby.internal.runtime.methods.*;
 import org.jruby.ir.JIT;
 import org.jruby.ir.operands.UndefinedValue;
@@ -17,15 +18,15 @@
 import org.jruby.runtime.Helpers;
 import org.jruby.runtime.ThreadContext;
 import org.jruby.runtime.builtin.IRubyObject;
-import org.jruby.runtime.invokedynamic.InvocationLinker;
-import org.jruby.runtime.invokedynamic.InvokeDynamicSupport;
+import org.jruby.runtime.invokedynamic.GlobalSite;
 import org.jruby.runtime.invokedynamic.MathLinker;
 import org.jruby.runtime.invokedynamic.VariableSite;
 import org.jruby.runtime.ivars.FieldVariableAccessor;
 import org.jruby.runtime.ivars.VariableAccessor;
 import org.jruby.runtime.opto.Invalidator;
 import org.jruby.runtime.opto.OptoFactory;
 import org.jruby.util.ByteList;
+import org.jruby.util.JavaNameMangler;
 import org.jruby.util.cli.Options;
 import org.jruby.util.log.Logger;
 import org.jruby.util.log.LoggerFactory;
@@ -179,6 +180,10 @@ public static Handle searchConst() {
         return new Handle(Opcodes.H_INVOKESTATIC, p(Bootstrap.class), "searchConst", sig(CallSite.class, Lookup.class, String.class, MethodType.class, int.class));
     }
 
+    public static Handle global() {
+        return new Handle(Opcodes.H_INVOKESTATIC, p(Bootstrap.class), "globalBootstrap", sig(CallSite.class, Lookup.class, String.class, MethodType.class));
+    }
+
     public static RubyString string(MutableCallSite site, ByteList value, int cr, ThreadContext context) throws Throwable {
         MethodHandle handle = SmartBinder
                 .from(STRING_SIGNATURE)
@@ -862,4 +867,61 @@ public static void checkpointFallback(MutableCallSite site, ThreadContext contex
 
         site.setTarget(target);
     }
+
+    public static CallSite globalBootstrap(Lookup lookup, String name, MethodType type) throws Throwable {
+        String[] names = name.split(":");
+        String operation = names[0];
+        String varName = JavaNameMangler.demangleMethodName(names[1]);
+        GlobalSite site = new GlobalSite(type, varName);
+        MethodHandle handle;
+
+        if (operation.equals("get")) {
+            handle = lookup.findStatic(Bootstrap.class, "getGlobalFallback", methodType(IRubyObject.class, GlobalSite.class, ThreadContext.class));
+        } else {
+            throw new RuntimeException("invalid variable access type");
+        }
+
+        handle = handle.bindTo(site);
+        site.setTarget(handle);
+
+        return site;
+    }
+
+    public static IRubyObject getGlobalFallback(GlobalSite site, ThreadContext context) throws Throwable {
+        Ruby runtime = context.runtime;
+        GlobalVariable variable = runtime.getGlobalVariables().getVariable(site.name());
+
+        if (site.failures() > Options.INVOKEDYNAMIC_GLOBAL_MAXFAIL.load() ||
+                variable.getScope() != GlobalVariable.Scope.GLOBAL) {
+
+            // use uncached logic forever
+//            if (Options.INVOKEDYNAMIC_LOG_GLOBALS.load()) LOG.info("global " + site.name() + " (" + site.file() + ":" + site.line() + ") rebound > " + Options.INVOKEDYNAMIC_GLOBAL_MAXFAIL.load() + " times, reverting to simple lookup");
+
+            MethodHandle uncached = lookup().findStatic(Bootstrap.class, "getGlobalUncached", methodType(IRubyObject.class, GlobalVariable.class));
+            uncached = uncached.bindTo(variable);
+            uncached = dropArguments(uncached, 0, ThreadContext.class);
+            site.setTarget(uncached);
+            return (IRubyObject)uncached.invokeWithArguments(context);
+        }
+
+        Invalidator invalidator = variable.getInvalidator();
+        IRubyObject value = variable.getAccessor().getValue();
+
+        MethodHandle target = constant(IRubyObject.class, value);
+        target = dropArguments(target, 0, ThreadContext.class);
+        MethodHandle fallback = lookup().findStatic(Bootstrap.class, "getGlobalFallback", methodType(IRubyObject.class, GlobalSite.class, ThreadContext.class));
+        fallback = fallback.bindTo(site);
+
+        target = ((SwitchPoint)invalidator.getData()).guardWithTest(target, fallback);
+
+        site.setTarget(target);
+
+//        if (Options.INVOKEDYNAMIC_LOG_GLOBALS.load()) LOG.info("global " + site.name() + " (" + site.file() + ":" + site.line() + ") cached");
+
+        return value;
+    }
+
+    public static IRubyObject getGlobalUncached(GlobalVariable variable) throws Throwable {
+        return variable.getAccessor().getValue();
+    }
 }
diff --git a/core/src/main/java/org/jruby/ir/targets/IRBytecodeAdapter.java b/core/src/main/java/org/jruby/ir/targets/IRBytecodeAdapter.java
@@ -496,6 +496,20 @@ public void pushBlockBody(Handle handle, org.jruby.runtime.Signature signature,
      */
     public abstract void checkpoint();
 
+    /**
+     * Retrieve a global variable with the given name.
+     *
+     * Stack required: none
+     */
+    public abstract void getGlobalVariable(String name);
+
+    /**
+     * Set the global variable with the given name to the value on stack.
+     *
+     * Stack required: the new value
+     */
+    public abstract void setGlobalVariable(String name);
+
     public SkinnyMethodAdapter adapter;
     private int variableCount = 0;
     private Map<Integer, Type> variableTypes = new HashMap<Integer, Type>();

diff --git a/core/src/main/java/org/jruby/ir/targets/IRBytecodeAdapter6.java b/core/src/main/java/org/jruby/ir/targets/IRBytecodeAdapter6.java
@@ -822,5 +822,21 @@ public void checkpoint() {
                 sig(void.class));
     }
 
+    @Override
+    public void getGlobalVariable(String name) {
+        loadContext();
+        adapter.invokedynamic(
+                "get:" + JavaNameMangler.mangleMethodName(name),
+                sig(IRubyObject.class, ThreadContext.class),
+                Bootstrap.global());
+    }
+
+    @Override
+    public void setGlobalVariable(String name) {
+        loadRuntime();
+        adapter.ldc(name);
+        invokeHelper("setGlobalVariable", sig(IRubyObject.class, IRubyObject.class, Ruby.class, String.class));
+    }
+
     private final Map<Object, String> cacheFieldNames = new HashMap<>();
 }
diff --git a/core/src/main/java/org/jruby/ir/targets/JVMVisitor.java b/core/src/main/java/org/jruby/ir/targets/JVMVisitor.java
@@ -21,7 +21,6 @@
 import org.jruby.ir.operands.*;
 import org.jruby.ir.operands.Boolean;
 import org.jruby.ir.operands.Float;
-import org.jruby.ir.operands.GlobalVariable;
 import org.jruby.ir.operands.Label;
 import org.jruby.ir.representations.BasicBlock;
 import org.jruby.ir.runtime.IRRuntimeHelpers;
@@ -1088,11 +1087,7 @@ public void GetFieldInstr(GetFieldInstr getfieldinstr) {
 
     @Override
     public void GetGlobalVariableInstr(GetGlobalVariableInstr getglobalvariableinstr) {
-        String name = getglobalvariableinstr.getGVar().getName();
-        jvmMethod().loadRuntime();
-        jvmMethod().invokeVirtual(Type.getType(Ruby.class), Method.getMethod("org.jruby.internal.runtime.GlobalVariables getGlobalVariables()"));
-        jvmAdapter().ldc(name);
-        jvmMethod().invokeVirtual(Type.getType(GlobalVariables.class), Method.getMethod("org.jruby.runtime.builtin.IRubyObject get(String)"));
+        jvmMethod().getGlobalVariable(getglobalvariableinstr.getTarget().getName());
         jvmStoreLocal(getglobalvariableinstr.getResult());
     }
 
@@ -1461,13 +1456,8 @@ public void PutFieldInstr(PutFieldInstr putfieldinstr) {
 
     @Override
     public void PutGlobalVarInstr(PutGlobalVarInstr putglobalvarinstr) {
-        GlobalVariable target = (GlobalVariable)putglobalvarinstr.getTarget();
-        String name = target.getName();
-        jvmMethod().loadRuntime();
-        jvmMethod().invokeVirtual(Type.getType(Ruby.class), Method.getMethod("org.jruby.internal.runtime.GlobalVariables getGlobalVariables()"));
-        jvmAdapter().ldc(name);
         visit(putglobalvarinstr.getValue());
-        jvmMethod().invokeVirtual(Type.getType(GlobalVariables.class), Method.getMethod("org.jruby.runtime.builtin.IRubyObject set(String, org.jruby.runtime.builtin.IRubyObject)"));
+        jvmMethod().setGlobalVariable(putglobalvarinstr.getTarget().getName());
         // leaves copy of value on stack
         jvmAdapter().pop();
     }

diff --git a/core/src/main/java/org/jruby/runtime/invokedynamic/GlobalSite.java b/core/src/main/java/org/jruby/runtime/invokedynamic/GlobalSite.java
@@ -0,0 +1,38 @@
+package org.jruby.runtime.invokedynamic;
+
+import java.lang.invoke.MethodHandle;
+import org.jruby.Ruby;
+import org.jruby.RubyClass;
+import org.jruby.common.IRubyWarnings;
+import org.jruby.runtime.ThreadContext;
+import org.jruby.runtime.builtin.IRubyObject;
+
+import java.lang.invoke.MethodType;
+import java.lang.invoke.MutableCallSite;
+
+public class GlobalSite extends MutableCallSite {
+    private final String name;
+    private volatile int failures;
+
+    public GlobalSite(MethodType type, String name) {
+        super(type);
+        this.name = name;
+    }
+
+    public void setTarget(MethodHandle target) {
+        super.setTarget(target);
+        incrementFailures();
+    }
+
+    public int failures() {
+        return failures;
+    }
+
+    public void incrementFailures() {
+        failures += 1;
+    }
+
+    public String name() {
+        return name;
+    }
+}
diff --git a/core/src/main/java/org/jruby/runtime/opto/OptoFactory.java b/core/src/main/java/org/jruby/runtime/opto/OptoFactory.java
@@ -67,17 +67,7 @@ private static Boolean indyEnabled() {
     }
 
     public static Invalidator newGlobalInvalidator(int maxFailures) {
-        if (indyEnabled() && indyConstants()) {
-            try {
-                return new FailoverSwitchPointInvalidator(maxFailures);
-            } catch (Error e) {
-                disableIndy();
-                throw e;
-            } catch (Throwable t) {
-                disableIndy();
-            }
-        }
-        return new ObjectIdentityInvalidator();
+        return new FailoverSwitchPointInvalidator(maxFailures);
     }
 
     public static Invalidator newMethodInvalidator(RubyModule module) {