[Truffle] Improve uncached dispatch.

jruby · Oct 30, 2014 · eed2a9e · eed2a9e
1 parent 18bb4db
commit eed2a9e
Showing 8 changed files with 147 additions and 38 deletions.
diff --git a/core/src/main/java/org/jruby/truffle/nodes/RubyNode.java b/core/src/main/java/org/jruby/truffle/nodes/RubyNode.java
@@ -59,6 +59,10 @@ public Object isDefined(VirtualFrame frame) {
         return getContext().makeString("expression");
     }
 
+    public String executeJavaString(VirtualFrame frame) throws UnexpectedResultException {
+        return RubyTypesGen.RUBYTYPES.expectString(execute(frame));
+    }
+
     public RubyArray executeArray(VirtualFrame frame) throws UnexpectedResultException {
         return RubyTypesGen.RUBYTYPES.expectRubyArray(execute(frame));
     }

diff --git a/core/src/main/java/org/jruby/truffle/nodes/RubyTypes.java b/core/src/main/java/org/jruby/truffle/nodes/RubyTypes.java
@@ -36,6 +36,7 @@
                 int.class, //
                 long.class, //
                 double.class, //
+                String.class, // for SymbolCastNode
                 BigInteger.class, //
                 RubyRange.IntegerFixnumRange.class, //
                 RubyRange.LongFixnumRange.class, //

diff --git a/core/src/main/java/org/jruby/truffle/nodes/conversion/ToJavaStringNode.java b/core/src/main/java/org/jruby/truffle/nodes/conversion/ToJavaStringNode.java
@@ -0,0 +1,54 @@
+/*
+ * Copyright (c) 2014 Oracle and/or its affiliates. All rights reserved. This
+ * code is released under a tri EPL/GPL/LGPL license. You can use it,
+ * redistribute it and/or modify it under the terms of the:
+ *
+ * Eclipse Public License version 1.0
+ * GNU General Public License version 2
+ * GNU Lesser General Public License version 2.1
+ */
+package org.jruby.truffle.nodes.conversion;
+
+import com.oracle.truffle.api.CompilerDirectives;
+import com.oracle.truffle.api.dsl.NodeChild;
+import com.oracle.truffle.api.dsl.Specialization;
+import com.oracle.truffle.api.frame.VirtualFrame;
+import com.oracle.truffle.api.source.SourceSection;
+import org.jruby.truffle.nodes.RubyNode;
+import org.jruby.truffle.runtime.RubyContext;
+import org.jruby.truffle.runtime.core.RubyString;
+import org.jruby.truffle.runtime.core.RubySymbol;
+
+@NodeChild(value="child", type=RubyNode.class)
+public abstract class ToJavaStringNode extends RubyNode {
+
+    public ToJavaStringNode(RubyContext context, SourceSection sourceSection) {
+        super(context, sourceSection);
+    }
+
+    public ToJavaStringNode(ToJavaStringNode prev) {
+        super(prev);
+    }
+
+    public abstract String executeJavaString(VirtualFrame frame, Object object);
+
+    // TODO(CS): cache the conversion to a Java String? Or should the user do that themselves?
+
+    @CompilerDirectives.SlowPath
+    @Specialization
+    protected String toJavaString(RubySymbol symbol) {
+        return symbol.toString();
+    }
+
+    @CompilerDirectives.SlowPath
+    @Specialization
+    protected String toJavaString(RubyString string) {
+        return string.toString();
+    }
+
+    @Specialization
+    protected String toJavaString(String string) {
+        return string;
+    }
+
+}
diff --git a/core/src/main/java/org/jruby/truffle/nodes/conversion/ToSymbolNode.java b/core/src/main/java/org/jruby/truffle/nodes/conversion/ToSymbolNode.java
@@ -0,0 +1,51 @@
+/*
+ * Copyright (c) 2014 Oracle and/or its affiliates. All rights reserved. This
+ * code is released under a tri EPL/GPL/LGPL license. You can use it,
+ * redistribute it and/or modify it under the terms of the:
+ *
+ * Eclipse Public License version 1.0
+ * GNU General Public License version 2
+ * GNU Lesser General Public License version 2.1
+ */
+package org.jruby.truffle.nodes.conversion;
+
+import com.oracle.truffle.api.dsl.NodeChild;
+import com.oracle.truffle.api.dsl.Specialization;
+import com.oracle.truffle.api.frame.VirtualFrame;
+import com.oracle.truffle.api.source.SourceSection;
+import org.jruby.truffle.nodes.RubyNode;
+import org.jruby.truffle.runtime.RubyContext;
+import org.jruby.truffle.runtime.core.RubyString;
+import org.jruby.truffle.runtime.core.RubySymbol;
+
+@NodeChild(value="child", type=RubyNode.class)
+public abstract class ToSymbolNode extends RubyNode {
+
+    public ToSymbolNode(RubyContext context, SourceSection sourceSection) {
+        super(context, sourceSection);
+    }
+
+    public ToSymbolNode(ToSymbolNode prev) {
+        super(prev);
+    }
+
+    public abstract RubySymbol executeRubySymbol(VirtualFrame frame, Object object);
+
+    // TODO(CS): cache the conversion to a symbol? Or should the user do that themselves?
+
+    @Specialization
+    protected RubySymbol toSymbol(RubySymbol symbol) {
+        return symbol;
+    }
+
+    @Specialization
+    protected RubySymbol toSymbol(RubyString string) {
+        return getContext().newSymbol(string.getBytes());
+    }
+
+    @Specialization
+    protected RubySymbol toSymbol(String string) {
+        return getContext().newSymbol(string);
+    }
+
+}
diff --git a/core/src/main/java/org/jruby/truffle/nodes/dispatch/CachedDispatchNode.java b/core/src/main/java/org/jruby/truffle/nodes/dispatch/CachedDispatchNode.java
@@ -11,6 +11,7 @@
 
 import com.oracle.truffle.api.dsl.Fallback;
 import com.oracle.truffle.api.frame.VirtualFrame;
+import com.oracle.truffle.api.utilities.BranchProfile;
 import org.jruby.truffle.runtime.LexicalScope;
 import org.jruby.truffle.runtime.RubyContext;
 import org.jruby.truffle.runtime.core.RubyBasicObject;
@@ -24,6 +25,8 @@ public abstract class CachedDispatchNode extends DispatchNode {
 
     @Child protected DispatchNode next;
 
+    private final BranchProfile moreThanReferenceCompare = new BranchProfile();
+
     public CachedDispatchNode(RubyContext context, Object cachedName, DispatchNode next) {
         super(context);
 
@@ -57,6 +60,12 @@ protected final boolean guardName(
             Object methodName,
             Object blockObject,
             Object argumentsObjects) {
+        if (cachedName == methodName) {
+            return true;
+        }
+
+        moreThanReferenceCompare.enter();
+
         if (cachedName instanceof String) {
             return cachedName.equals(methodName);
         } else if (cachedName instanceof RubySymbol) {

diff --git a/core/src/main/java/org/jruby/truffle/nodes/dispatch/DispatchNode.java b/core/src/main/java/org/jruby/truffle/nodes/dispatch/DispatchNode.java
@@ -53,14 +53,13 @@ public abstract Object executeDispatch(
             Object argumentsObjects,
             Dispatch.DispatchAction dispatchAction);
 
+    @CompilerDirectives.SlowPath
     protected RubyConstant lookupConstant(
             LexicalScope lexicalScope,
             RubyModule module,
             String name,
             boolean ignoreVisibility,
             Dispatch.DispatchAction dispatchAction) {
-        CompilerAsserts.neverPartOfCompilation();
-
         return ModuleOperations.lookupConstant(lexicalScope, module, name);
     }
 

diff --git a/core/src/main/java/org/jruby/truffle/nodes/dispatch/UncachedDispatchNode.java b/core/src/main/java/org/jruby/truffle/nodes/dispatch/UncachedDispatchNode.java
@@ -9,13 +9,17 @@
  */
 package org.jruby.truffle.nodes.dispatch;
 
-import com.oracle.truffle.api.CompilerAsserts;
 import com.oracle.truffle.api.dsl.Specialization;
 import com.oracle.truffle.api.frame.VirtualFrame;
 import com.oracle.truffle.api.nodes.IndirectCallNode;
 import com.oracle.truffle.api.CompilerDirectives;
 import com.oracle.truffle.api.Truffle;
+import com.oracle.truffle.api.utilities.BranchProfile;
 import org.jruby.truffle.nodes.cast.BoxingNode;
+import org.jruby.truffle.nodes.conversion.ToJavaStringNode;
+import org.jruby.truffle.nodes.conversion.ToJavaStringNodeFactory;
+import org.jruby.truffle.nodes.conversion.ToSymbolNode;
+import org.jruby.truffle.nodes.conversion.ToSymbolNodeFactory;
 import org.jruby.truffle.runtime.RubyArguments;
 import org.jruby.truffle.runtime.RubyConstant;
 import org.jruby.truffle.runtime.RubyContext;
@@ -30,19 +34,28 @@ public abstract class UncachedDispatchNode extends DispatchNode {
 
     @Child protected IndirectCallNode callNode;
     @Child protected BoxingNode box;
+    @Child protected ToSymbolNode toSymbolNode;
+    @Child protected ToJavaStringNode toJavaStringNode;
+
+    private final BranchProfile constantMissingProfile = new BranchProfile();
+    private final BranchProfile methodMissingProfile = new BranchProfile();
 
     public UncachedDispatchNode(RubyContext context, boolean ignoreVisibility) {
         super(context);
         this.ignoreVisibility = ignoreVisibility;
         callNode = Truffle.getRuntime().createIndirectCallNode();
         box = new BoxingNode(context, null, null);
+        toSymbolNode = ToSymbolNodeFactory.create(context, null, null);
+        toJavaStringNode = ToJavaStringNodeFactory.create(context, null, null);
     }
 
     public UncachedDispatchNode(UncachedDispatchNode prev) {
         super(prev);
         ignoreVisibility = prev.ignoreVisibility;
         callNode = prev.callNode;
         box = prev.box;
+        toSymbolNode = prev.toSymbolNode;
+        toJavaStringNode = prev.toJavaStringNode;
     }
 
     @Specialization(guards = "actionIsReadConstant")
@@ -51,42 +64,30 @@ public Object dispatchReadConstant(
             Object methodReceiverObject,
             LexicalScope lexicalScope,
             RubyModule receiverObject,
-            Object methodName,
+            Object constantName,
             Object blockObject,
             Object argumentsObjects,
             Dispatch.DispatchAction dispatchAction) {
-        // Need to be much more fine grained with TruffleBoundary here
-        CompilerDirectives.transferToInterpreter();
-
         final RubyConstant constant = lookupConstant(lexicalScope, receiverObject,
-                methodName.toString(), ignoreVisibility, dispatchAction);
+                toJavaStringNode.executeJavaString(frame, constantName), ignoreVisibility, dispatchAction);
 
         if (constant != null) {
             return constant.getValue();
         }
 
+        constantMissingProfile.enter();
+
         final RubyClass callerClass = ignoreVisibility ? null : box.box(RubyArguments.getSelf(frame.getArguments())).getMetaClass();
 
         final RubyMethod missingMethod = lookup(callerClass, receiverObject, "const_missing", ignoreVisibility,
                 dispatchAction);
 
         if (missingMethod == null) {
+            CompilerDirectives.transferToInterpreter();
             throw new RaiseException(getContext().getCoreLibrary().runtimeError(
                     receiverObject.toString() + " didn't have a #const_missing", this));
         }
 
-        final RubySymbol methodNameAsSymbol;
-
-        if (methodName instanceof RubySymbol) {
-            methodNameAsSymbol = (RubySymbol) methodName;
-        } else if (methodName instanceof RubyString) {
-            methodNameAsSymbol = getContext().newSymbol(((RubyString) methodName).getBytes());
-        } else if (methodName instanceof String) {
-            methodNameAsSymbol = getContext().newSymbol((String) methodName);
-        } else {
-            throw new UnsupportedOperationException();
-        }
-
         return callNode.call(
                 frame,
                 missingMethod.getCallTarget(),
@@ -95,7 +96,7 @@ public Object dispatchReadConstant(
                         missingMethod.getDeclarationFrame(),
                         receiverObject,
                         null,
-                        new Object[]{methodNameAsSymbol}));
+                        new Object[]{toSymbolNode.executeRubySymbol(frame, constantName)}));
     }
 
     @Specialization(guards = "actionIsCallOrRespondToMethod")
@@ -108,12 +109,9 @@ public Object dispatch(
             Object blockObject,
             Object argumentsObjects,
             Dispatch.DispatchAction dispatchAction) {
-        // Need to be much more fine grained with TruffleBoundary here
-        CompilerDirectives.transferToInterpreter();
-
         final RubyClass callerClass = ignoreVisibility ? null : box.box(RubyArguments.getSelf(frame.getArguments())).getMetaClass();
 
-        final RubyMethod method = lookup(callerClass, receiverObject, methodName.toString(),
+        final RubyMethod method = lookup(callerClass, receiverObject, toJavaStringNode.executeJavaString(frame, methodName),
                 ignoreVisibility, dispatchAction);
 
         if (method != null) {
@@ -134,13 +132,16 @@ public Object dispatch(
             }
         }
 
+        methodMissingProfile.enter();
+
         final RubyMethod missingMethod = lookup(callerClass, receiverObject, "method_missing", true,
                 dispatchAction);
 
         if (missingMethod == null) {
             if (dispatchAction == Dispatch.DispatchAction.RESPOND_TO_METHOD) {
                 return false;
             } else {
+                CompilerDirectives.transferToInterpreter();
                 throw new RaiseException(getContext().getCoreLibrary().runtimeError(
                         receiverObject.toString() + " didn't have a #method_missing", this));
             }
@@ -151,19 +152,7 @@ public Object dispatch(
 
             final Object[] modifiedArgumentsObjects = new Object[1 + argumentsObjectsArray.length];
 
-            final RubySymbol methodNameAsSymbol;
-
-            if (methodName instanceof RubySymbol) {
-                methodNameAsSymbol = (RubySymbol) methodName;
-            } else if (methodName instanceof RubyString) {
-                methodNameAsSymbol = getContext().newSymbol(((RubyString) methodName).getBytes());
-            } else if (methodName instanceof String) {
-                methodNameAsSymbol = getContext().newSymbol((String) methodName);
-            } else {
-                throw new UnsupportedOperationException();
-            }
-
-            modifiedArgumentsObjects[0] = methodNameAsSymbol;
+            modifiedArgumentsObjects[0] = toSymbolNode.executeRubySymbol(frame, methodName);
 
             System.arraycopy(argumentsObjectsArray, 0, modifiedArgumentsObjects, 1, argumentsObjectsArray.length);
 

diff --git a/core/src/main/java/org/jruby/truffle/runtime/core/RubySymbol.java b/core/src/main/java/org/jruby/truffle/runtime/core/RubySymbol.java
@@ -112,6 +112,8 @@ public RubySymbol getSymbol(String name) {
         }
 
         public RubySymbol getSymbol(ByteList byteList) {
+            // TODO(CS): is this broken? ByteList is mutable...
+
             RubySymbol symbol = symbolsTable.get(byteList);
 
             if (symbol == null) {