Cache the Symbol of the method name in dispatch nodes only when needed

Author: eregon

CHANGELOG.md

@@ -76,6 +76,7 @@ Compatibility:
 * Implemented `Enumerator::Chain`, `Enumerator#+`, and `Enumerable#chain` (#1859, #1858).
 * Implemented `Thread#backtrace_locations` and `Exception#backtrace_locations` (#1556).
 * Implemented `rb_module_new`, `rb_define_class_id`, `rb_define_module_id`, (#1876, @chrisseaton).
+* Cache the `Symbol` of method names in call nodes only when needed (#1872).
 
 Performance:
 

src/main/java/org/truffleruby/language/dispatch/CachedBoxedDispatchNode.java

@@ -108,7 +108,7 @@ public Object executeDispatch(
     public String toString() {
         return StringUtils.format(
                 "CachedBoxedDispatchNode(:%s, %x, %s)",
-                getCachedNameAsSymbol().toString(),
+                getCachedName().toString(),
                 expectedShape.hashCode(),
                 method == null ? "null" : method.toString());
     }

src/main/java/org/truffleruby/language/dispatch/CachedDispatchNode.java

@@ -30,7 +30,6 @@
 public abstract class CachedDispatchNode extends DispatchNode {
 
     private final Object cachedName;
-    private final DynamicObject cachedNameAsSymbol;
 
     @Child protected DispatchNode next;
     @Child private RopeNodes.BytesEqualNode ropeEqualsNode;
@@ -48,15 +47,8 @@ public CachedDispatchNode(
                 (RubyGuards.isRubyString(cachedName));
         this.cachedName = cachedName;
 
-        if (RubyGuards.isRubySymbol(cachedName)) {
-            cachedNameAsSymbol = (DynamicObject) cachedName;
-        } else if (RubyGuards.isRubyString(cachedName)) {
-            cachedNameAsSymbol = context.getSymbolTable().getSymbol(StringOperations.rope((DynamicObject) cachedName));
+        if (RubyGuards.isRubyString(cachedName)) {
             ropeEqualsNode = RopeNodes.BytesEqualNode.create();
-        } else if (cachedName instanceof String) {
-            cachedNameAsSymbol = context.getSymbolTable().getSymbol((String) cachedName);
-        } else {
-            throw new UnsupportedOperationException();
         }
 
         this.next = next;
@@ -97,8 +89,8 @@ protected final boolean guardName(Object methodName) {
         }
     }
 
-    protected DynamicObject getCachedNameAsSymbol() {
-        return cachedNameAsSymbol;
+    public Object getCachedName() {
+        return cachedName;
     }
 
     protected abstract void reassessSplittingInliningStrategy();

src/main/java/org/truffleruby/language/dispatch/CachedMethodMissingDispatchNode.java

@@ -12,6 +12,8 @@
 import org.truffleruby.RubyContext;
 import org.truffleruby.core.array.ArrayUtils;
 import org.truffleruby.core.module.MethodLookupResult;
+import org.truffleruby.core.string.StringOperations;
+import org.truffleruby.language.RubyGuards;
 import org.truffleruby.language.methods.InternalMethod;
 import org.truffleruby.language.objects.MetaClassNode;
 
@@ -34,6 +36,8 @@ public class CachedMethodMissingDispatchNode extends CachedDispatchNode {
     @Child private MetaClassNode metaClassNode;
     @Child private DirectCallNode callNode;
 
+    private final DynamicObject cachedNameAsSymbol;
+
     public CachedMethodMissingDispatchNode(
             RubyContext context,
             Object cachedName,
@@ -50,6 +54,16 @@ public CachedMethodMissingDispatchNode(
         this.methodMissing = methodMissingLookup.getMethod();
         this.metaClassNode = MetaClassNode.create();
         this.callNode = Truffle.getRuntime().createDirectCallNode(methodMissing.getCallTarget());
+
+        if (RubyGuards.isRubySymbol(cachedName)) {
+            cachedNameAsSymbol = (DynamicObject) cachedName;
+        } else if (RubyGuards.isRubyString(cachedName)) {
+            cachedNameAsSymbol = context.getSymbolTable().getSymbol(StringOperations.rope((DynamicObject) cachedName));
+        } else if (cachedName instanceof String) {
+            cachedNameAsSymbol = context.getSymbolTable().getSymbol((String) cachedName);
+        } else {
+            throw new UnsupportedOperationException();
+        }
     }
 
     @Override
@@ -114,7 +128,7 @@ public Object executeDispatch(
         switch (getDispatchAction()) {
             case CALL_METHOD:
                 // When calling #method_missing we need to prepend the symbol
-                final Object[] modifiedArgumentsObjects = ArrayUtils.unshift(argumentsObjects, getCachedNameAsSymbol());
+                final Object[] modifiedArgumentsObjects = ArrayUtils.unshift(argumentsObjects, cachedNameAsSymbol);
 
                 return call(callNode, frame, methodMissing, receiverObject, blockObject, modifiedArgumentsObjects);
 

src/main/java/org/truffleruby/language/dispatch/CachedSingletonDispatchNode.java

@@ -107,7 +107,7 @@ public Object executeDispatch(
     public String toString() {
         return StringUtils.format(
                 "CachedBoxedDispatchNode(:%s, %s@%x, %s)",
-                getCachedNameAsSymbol().toString(),
+                getCachedName().toString(),
                 expectedReceiver,
                 expectedReceiver.hashCode(),
                 method == null ? "null" : method.toString());