Merge pull request jruby#8259 from headius/better_debug_jit

Improved representation of JRuby for JVM tools

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

@@ -1349,10 +1349,10 @@ protected boolean hasInstanceVariables() {
      */
     @Override
     public List<Variable<Object>> getVariableList() {
-        Map<String, VariableAccessor> ivarAccessors = metaClass.getVariableAccessorsForRead();
-        ArrayList<Variable<Object>> list = new ArrayList<>(ivarAccessors.size());
-        for (Map.Entry<String, VariableAccessor> entry : ivarAccessors.entrySet()) {
-            Object value = entry.getValue().get(this);
+        var ivarAccessors = metaClass.getVariableAccessorsForRead();
+        var list = new ArrayList<Variable<Object>>(ivarAccessors.size());
+        for (var entry : ivarAccessors.entrySet()) {
+            var value = entry.getValue().get(this);
             if (value == null) continue;
             list.add(new VariableEntry<>(entry.getKey(), value));
         }
@@ -1363,10 +1363,10 @@ public List<Variable<Object>> getVariableList() {
      * @see IRubyObject#getMarshalVariableList()
      */
     public List<Variable<Object>> getMarshalVariableList() {
-        Map<String, VariableAccessor> ivarAccessors = metaClass.getVariableAccessorsForRead();
-        ArrayList<Variable<Object>> list = new ArrayList<>(ivarAccessors.size());
+        var ivarAccessors = metaClass.getVariableAccessorsForRead();
+        var list = new ArrayList<Variable<Object>>(ivarAccessors.size());
         for (Map.Entry<String, VariableAccessor> entry : ivarAccessors.entrySet()) {
-            Object value = entry.getValue().get(this);
+            var value = entry.getValue().get(this);
             if (value == null || !(value instanceof Serializable)) continue;
             list.add(new VariableEntry<>(entry.getKey(), value));
         }
@@ -1378,10 +1378,10 @@ public List<Variable<Object>> getMarshalVariableList() {
      */
     @Override
     public List<String> getVariableNameList() {
-        Map<String, VariableAccessor> ivarAccessors = metaClass.getVariableAccessorsForRead();
-        ArrayList<String> list = new ArrayList<>(ivarAccessors.size());
-        for (Map.Entry<String, VariableAccessor> entry : ivarAccessors.entrySet()) {
-            Object value = entry.getValue().get(this);
+        var ivarAccessors = metaClass.getVariableAccessorsForRead();
+        var list = new ArrayList<String>(ivarAccessors.size());
+        for (var entry : ivarAccessors.entrySet()) {
+            var value = entry.getValue().get(this);
             if (value == null) continue;
             list.add(entry.getKey());
         }

core/src/main/java/org/jruby/compiler/impl/SkinnyMethodAdapter.java

@@ -642,6 +642,10 @@ public void local(int index, String name, Type type) {
         getMethodVisitor().visitLocalVariable(name, type.getDescriptor(), null, start, end, index);
     }
 
+    public void local(int index, String name, Type type, Label start) {
+        getMethodVisitor().visitLocalVariable(name, type.getDescriptor(), null, start, end, index);
+    }
+
     public void line(int line) {
         Label label = new Label();
         label(label);

core/src/main/java/org/jruby/ir/operands/TemporaryLocalReplacementVariable.java

@@ -25,7 +25,12 @@ public Variable clone(SimpleCloneInfo ii) {
 
     @Override
     public String getPrefix() {
-        return PREFIX + oldName + "_";
+        return oldName;
+    }
+
+    @Override
+    public String getId() {
+        return oldName;
     }
 
     @Override

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

@@ -42,6 +42,7 @@
 import org.jruby.runtime.Helpers;
 import org.jruby.runtime.builtin.IRubyObject;
 import org.jruby.util.JavaNameMangler;
+import org.jruby.util.cli.Options;
 import org.jruby.util.collections.IntHashMap;
 import org.objectweb.asm.Handle;
 import org.objectweb.asm.Label;
@@ -50,6 +51,8 @@
 import org.objectweb.asm.commons.Method;
 
 import java.lang.invoke.MethodType;
+import java.util.HashMap;
+import java.util.Map;
 
 import static org.jruby.util.CodegenUtils.*;
 
@@ -255,15 +258,17 @@ public void startMethod() {
     }
 
     public void endMethod() {
-        adapter.end(new Runnable() {
-            public void run() {
-                for (IntHashMap.Entry<Type> entry : variableTypes.entrySet()) {
-                    final int i = entry.getKey();
-                    String name = variableNames.get(i);
-                    adapter.local(i, name, entry.getValue());
+        adapter.end(() -> variableTypes.forEach((i, type) -> {
+            String name = variableNames.get(i);
+            if (!name.startsWith("$") || Options.JIT_DEBUG.load()) {
+                Label varStart = varStarts.get(i);
+                if (varStart == null) {
+                    adapter.local(i, name, type);
+                } else {
+                    adapter.local(i, name, type, varStart);
                 }
             }
-        });
+        }));
     }
 
     public void loadLocal(int i) {
@@ -344,7 +349,14 @@ public void storeArgs() {
         adapter.astore(signature.argOffset("args"));
     }
 
+    private Map<Integer, Label> varStarts = new HashMap<>();
+
     public void storeLocal(int i) {
+        varStarts.computeIfAbsent(i, (Integer) -> {
+            Label varStart = newLabel();
+            adapter.label(varStart);
+            return varStart;
+        });
         adapter.astore(i);
     }
 

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

@@ -97,9 +97,10 @@
 public class JVMVisitor extends IRVisitor {
 
     private static final Logger LOG = LoggerFactory.getLogger(JVMVisitor.class);
-    public static final String DYNAMIC_SCOPE = "$dynamicScope";
+    public static final String DYNAMIC_SCOPE = "variableStore";
     private static final boolean DEBUG = false;
     public static final String BLOCK_ARG_NAME = "blockArg";
+    public static final String BLOCK_ARG_LOCAL_NAME = "&";
     public static final String SELF_BLOCK_NAME = "selfBlock";
     public static final String SUPER_NAME_NAME = "superName";
 
@@ -218,7 +219,7 @@ protected void emitScope(IRScope scope, String name, Signature signature, boolea
 
         if (fullIC.needsBinding() || !fullIC.hasExplicitCallProtocol()) {
             // declare dynamic scope local only if we'll need it
-            jvm.methodData().local("$dynamicScope", Type.getType(DynamicScope.class));
+            jvm.methodData().local(DYNAMIC_SCOPE, Type.getType(DynamicScope.class));
         }
 
         if (!fullIC.hasExplicitCallProtocol()) {
@@ -508,14 +509,14 @@ private void defineRunMethod(IRScriptBody script, String scopeName, String scope
     }
 
     protected void emitMethod(IRMethod method, JVMVisitorMethodContext context) {
-        String name = JavaNameMangler.encodeScopeForBacktrace(method) + '$' + methodIndex++;
+        String name = JavaNameMangler.encodeNumberedScopeForBacktrace(method, methodIndex++);
 
         emitWithSignatures(method, context, name);
     }
 
     protected void emitMethodJIT(IRMethod method, JVMVisitorMethodContext context) {
         String clsName = jvm.scriptToClass(method.getFile());
-        String name = JavaNameMangler.encodeScopeForBacktrace(method) + '$' + methodIndex++;
+        String name = JavaNameMangler.encodeNumberedScopeForBacktrace(method, methodIndex++);
         jvm.pushscript(this, clsName, method.getFile());
 
         emitWithSignatures(method, context, name);
@@ -526,7 +527,7 @@ protected void emitMethodJIT(IRMethod method, JVMVisitorMethodContext context) {
 
     protected void emitBlockJIT(IRClosure closure, JVMVisitorMethodContext context) {
         String clsName = jvm.scriptToClass(closure.getFile());
-        String name = JavaNameMangler.encodeScopeForBacktrace(closure) + '$' + methodIndex++;
+        String name = JavaNameMangler.encodeNumberedScopeForBacktrace(closure, methodIndex++);
         jvm.pushscript(this, clsName, closure.getFile());
 
         emitScope(closure, name, CLOSURE_SIGNATURE, false, true);
@@ -568,7 +569,7 @@ private void emitWithSignatures(IRMethod method, JVMVisitorMethodContext context
     }
 
     protected Handle emitModuleBodyJIT(IRModuleBody method) {
-        String name = JavaNameMangler.encodeScopeForBacktrace(method) + '$' + methodIndex++;
+        String name = JavaNameMangler.encodeNumberedScopeForBacktrace(method, methodIndex++);
 
         String clsName = jvm.scriptToClass(method.getFile());
         jvm.pushscript(this, clsName, method.getFile());
@@ -598,7 +599,7 @@ private void emitClosures(IRScope s, boolean print) {
 
     protected Handle emitClosure(IRClosure closure, boolean print) {
         /* Compile the closure like a method */
-        String name = JavaNameMangler.encodeScopeForBacktrace(closure) + '$' + methodIndex++;
+        String name = JavaNameMangler.encodeNumberedScopeForBacktrace(closure, methodIndex++);
 
         emitScope(closure, name, CLOSURE_SIGNATURE, false, print);
 
@@ -615,7 +616,7 @@ protected Handle emitClosure(IRClosure closure, boolean print) {
     }
 
     protected Handle emitModuleBody(IRModuleBody method) {
-        String name = JavaNameMangler.encodeScopeForBacktrace(method) + '$' + methodIndex++;
+        String name = JavaNameMangler.encodeNumberedScopeForBacktrace(method, methodIndex++);
 
         Signature signature = signatureFor(method, false);
         emitScope(method, name, signature, false, true);
@@ -2131,7 +2132,7 @@ public void PutGlobalVarInstr(PutGlobalVarInstr putglobalvarinstr) {
     @Override
     public void ReifyClosureInstr(ReifyClosureInstr reifyclosureinstr) {
         jvmMethod().loadContext();
-        jvmLoadLocal("$blockArg");
+        jvmLoadLocal(BLOCK_ARG_LOCAL_NAME);
         jvmMethod().invokeIRHelper("newProc", sig(IRubyObject.class, ThreadContext.class, Block.class));
         jvmStoreLocal(reifyclosureinstr.getResult());
     }

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

@@ -30,7 +30,13 @@ public MethodData(IRBytecodeAdapter method, IRScope scope, String scopeField, Si
 
         // incoming arguments
         for (int i = 0; i < signature.argCount(); i++) {
-            local("$" + signature.argName(i), Type.getType(signature.argType(i)));
+            String argName = signature.argName(i);
+            argName = switch (argName) {
+                case "self" -> "self";
+                case "blockArg" -> JVMVisitor.BLOCK_ARG_LOCAL_NAME;
+                default -> "$" + argName;
+            };
+            local(argName, Type.getType(signature.argType(i)));
         }
     }
 

core/src/main/java/org/jruby/runtime/builtin/IRubyObject.java

@@ -366,12 +366,12 @@ default IRubyObject dup(ThreadContext context) {
     void syncVariables(IRubyObject source);
 
     /**
-     * @return a list of all variables (ivar/cvar/constant/internal)
+     * @return a list of all variables (ivar/internal)
      */
     List<Variable<Object>> getVariableList();
 
     /**
-     * @return a list of all marshalable variables (ivar/cvar/constant/internal)
+     * @return a mutable list of all marshalable variables (ivar/internal)
      */
     default List<Variable<Object>> getMarshalVariableList() {
         return getVariableList();

core/src/main/java/org/jruby/util/JavaNameMangler.java

@@ -281,43 +281,60 @@ private static char unescapeChar(char character) {
         return DANGEROUS_CHARS.charAt(REPLACEMENT_CHARS.indexOf(character));
     }
 
+    private static final String RUBY_MARKER = "️❤";
+    private static final String METHOD_MARKER = "def";
+    private static final String BLOCK_MARKER = "{}";
+    private static final String METACLASS_MARKER = "metaclass";
+    private static final String CLASS_MARKER = "class";
+    private static final String MODULE_MARKER = "module";
+    private static final String SCRIPT_MARKER = "script";
+    private static final char DELIMITER = '\u00A0';
+
+    public static final String SCRIPT_METHOD_NAME = RUBY_MARKER + DELIMITER + SCRIPT_MARKER;
+
+    public static String encodeNumberedScopeForBacktrace(IRScope scope, int number) {
+        return encodeScopeForBacktrace(scope) + DELIMITER + '#' + number;
+    }
+
     // FIXME: bytelist_love - if we want these mangled names to display properly we should be building this up with encoded data.
     public static String encodeScopeForBacktrace(IRScope scope) {
+        String base;
+
         if (scope instanceof IRMethod) {
-            return "RUBY$method$" + mangleMethodNameInternal(scope.getId());
-        }
-        if (scope instanceof IRClosure) {
+            base = RUBY_MARKER + DELIMITER + METHOD_MARKER + DELIMITER + mangleMethodNameInternal(scope.getId());
+        } else if (scope instanceof IRClosure) {
             IRScope ancestorScope = scope.getNearestTopLocalVariableScope();
             String name;
             if (ancestorScope instanceof IRScriptBody) {
                 name = Interpreter.ROOT;
             } else {
                 name = ancestorScope.getId();
             }
-            return "RUBY$block$" + mangleMethodNameInternal(name);
-        }
-        if (scope instanceof IRMetaClassBody) {
-            return "RUBY$metaclass";
-        }
-        if (scope instanceof IRClassBody) {
-            return "RUBY$class$" + mangleMethodNameInternal(scope.getId());
-        }
-        if (scope instanceof IRModuleBody) {
-            return "RUBY$module$" + mangleMethodNameInternal(scope.getId());
-        }
-        if (scope instanceof IRScriptBody) {
-            return "RUBY$script";
+            base = RUBY_MARKER + DELIMITER + BLOCK_MARKER + DELIMITER + mangleMethodNameInternal(name);
+        } else if (scope instanceof IRMetaClassBody) {
+            base = RUBY_MARKER + DELIMITER + METACLASS_MARKER;
+        } else if (scope instanceof IRClassBody) {
+            base = RUBY_MARKER + DELIMITER + CLASS_MARKER + DELIMITER + mangleMethodNameInternal(scope.getId());
+        } else if (scope instanceof IRModuleBody) {
+            base = RUBY_MARKER + DELIMITER + MODULE_MARKER + DELIMITER + mangleMethodNameInternal(scope.getId());
+        } else if (scope instanceof IRScriptBody) {
+            base = SCRIPT_METHOD_NAME;
+        } else {
+            throw new IllegalStateException("unknown scope type for backtrace encoding: " + scope.getClass());
         }
-        throw new IllegalStateException("unknown scope type for backtrace encoding: " + scope.getClass());
+
+        // line is insufficient to guarantee a unique name
+//        return base + DELIMITER + '#' + scope.getLine();
+        return base;
     }
 
-    public static final String VARARGS_MARKER = "$__VARARGS__";
+    public static final String VARARGS_MARKER = DELIMITER + "**";
 
     // returns location $ type $ methodName as 3 elements or null if this is an invalid mangled name
     public static List<String> decodeMethodTuple(String methodName) {
-        if (!methodName.startsWith("RUBY$")) return null;
+        if (!methodName.startsWith(RUBY_MARKER + DELIMITER)) return null;
 
-        return StringSupport.split(methodName, '$');
+        return StringSupport.split(methodName, DELIMITER);
     }
 
     public static String decodeMethodName(FrameType type, List<String> mangledTuple) {
@@ -336,12 +353,12 @@ public static String decodeMethodName(FrameType type, List<String> mangledTuple)
 
     public static FrameType decodeFrameTypeFromMangledName(String type) {
         switch (type) {
-            case "script":    return FrameType.ROOT;
-            case "metaclass": return FrameType.METACLASS;
-            case "method":    return FrameType.METHOD;
-            case "block":     return FrameType.BLOCK;
-            case "class":     return FrameType.MODULE;
-            case "module":    return FrameType.CLASS;
+            case SCRIPT_MARKER:    return FrameType.ROOT;
+            case METACLASS_MARKER: return FrameType.METACLASS;
+            case METHOD_MARKER:    return FrameType.METHOD;
+            case BLOCK_MARKER:     return FrameType.BLOCK;
+            case CLASS_MARKER:     return FrameType.MODULE;
+            case MODULE_MARKER:    return FrameType.CLASS;
         }
         throw new IllegalStateException("unknown encoded method type '" + type);
 

core/src/main/java/org/jruby/util/cli/Options.java

@@ -97,6 +97,7 @@ public class Options {
     public static final Option<Boolean> JIT_BACKGROUND = bool(JIT, "jit.background", JIT_THRESHOLD.load() != 0, "Run the JIT compiler in a background thread. Off if jit.threshold=0.");
     public static final Option<Boolean> JIT_KERNEL = bool(JIT, "jit.kernel", false, "Run the JIT compiler while the pure-Ruby kernel is booting.");
     public static final Option<ClassLoaderMode> JIT_LOADER_MODE = enumeration(JIT, "jit.loader.mode", ClassLoaderMode.class, ClassLoaderMode.UNIQUE, "Set JIT class loader to use. UNIQUE class loader per class; SHARED loader for all classes");
+    public static final Option<Boolean> JIT_DEBUG = bool(JIT, "jit.debug", false, "Emit extra JIT information for debugging in JVM.");
 
     public static final Option<String> IR_DEBUG_IGV          = string(IR, "ir.debug.igv", (String) null, "Specify file:line of scope to jump to IGV");
     public static final Option<Boolean> IR_DEBUG_IGV_STDOUT = bool(IR, "ir.debug.igv.stdout", false, "Save IGV generated XML to stdout");
@@ -410,9 +411,6 @@ static void addPropertyNames(final Set<String> propertyNames) {
     @Deprecated
     public static final Option<Boolean> JIT_DUMPING = bool(JIT, "jit.dumping", false, "Enable stdout dumping of JITed bytecode.");
 
-    @Deprecated
-    public static final Option<Boolean> JIT_DEBUG = bool(JIT, "jit.debug", false, "Log loading of JITed bytecode.");
-
     @Deprecated
     public static final Option<String>  IR_INLINE_COMPILER_PASSES = string(IR, "ir.inline_passes", "Specify comma delimeted list of passes to run after inlining a method.");
 

core/src/main/java/org/jruby/util/collections/IntHashMap.java

@@ -2,10 +2,12 @@
 
 import java.util.AbstractCollection;
 import java.util.AbstractSet;
+import java.util.Arrays;
 import java.util.Collection;
 import java.util.Iterator;
 import java.util.NoSuchElementException;
 import java.util.Set;
+import java.util.function.BiConsumer;
 
 public class IntHashMap<V> {
 
@@ -410,6 +412,15 @@ public Object clone() {
         return newMap;
     }
 
+    public void forEach(BiConsumer<Integer, ? super V> action) {
+        for (Entry<V> entry : table) {
+            while (entry != null) {
+                action.accept(entry.getKey(), entry.value);
+                entry = entry.next;
+            }
+        }
+    }
+
     @SuppressWarnings("unchecked")
     public static <U> IntHashMap<U> nullMap() { return NullMap.INSTANCE; }
 

spec/compiler/general_spec.rb

@@ -96,7 +96,8 @@ def compile_to_method(src, filename, lineno)
 
     compiler = new_visitor(runtime)
     compiled = compiler.compile(method, org.jruby.util.OneShotClassLoader.new(runtime.getJRubyClassLoader()))
-    scriptMethod = compiled.getMethod("RUBY$script",
+    scriptMethod = compiled.getMethod(
+        org.jruby.util.JavaNameMangler::SCRIPT_METHOD_NAME,
         org.jruby.runtime.ThreadContext.java_class,
         org.jruby.parser.StaticScope.java_class,
         org.jruby.runtime.builtin.IRubyObject.java_class,