Reflectoring of message sending dispatch chains to simplify guards

The refactoring is general cleanup, removal of code duplication, but also a change in how guards in the dispatch chain and the chain itself works.

Now, we have simple unstructured chain. Before, the chain was structured, by having first all primitives, then a test node, and then all object types. Don't think this was really a working thing, because the type check was costing in the interpreter at every object check again, because of the checked cast.
In the compiled code, the check was bad, because it was not a leaf-type, and thus, required code for traversing the type hierarchy. This is now gone, we try to have guards and checked casts only on leaf-types to reduce the cost of guards.

In the interpreter, this hopefully also reduces the cost of dispatching. However guards are now in their own classes `DispatchGuard`, which means there is an extra virtual dispatch, which might be an issue for interpreter performance.

Further changes include the use of the object layout for objects with fields as guard critierion. This means, we can hopefully move more of the guards into the dispatch chain, and avoid duplicated guards. This however also means that handling invalidation of object layouts is part of this chain. Objects with old layouts are handled in the uninitialized node of the dispatch chain. After transfering an object to the new layout, lookup is retried to avoid adding duplicate entries to the dispatch chain.

Super, lexically bound, and receiver sends are now all handled with the same uninitialized node hierarchy, to avoid incomplete implementation (missing #dnu support for super, missing dispatch chain length, etc)

Signed-off-by: Stefan Marr <git@stefan-marr.de>

Author: smarr

src/som/compiler/MixinDefinition.java

@@ -21,10 +21,12 @@
 import som.interpreter.nodes.SlotAccessNode.SlotReadNode;
 import som.interpreter.nodes.SlotAccessNode.SlotWriteNode;
 import som.interpreter.nodes.dispatch.AbstractDispatchNode;
-import som.interpreter.nodes.dispatch.CachedSlotAccessNode;
-import som.interpreter.nodes.dispatch.CachedSlotAccessNode.CachedSlotWriteNode;
-import som.interpreter.nodes.dispatch.CachedSlotAccessNode.CheckedCachedSlotAccessNode;
-import som.interpreter.nodes.dispatch.CachedSlotAccessNode.CheckedCachedSlotWriteNode;
+import som.interpreter.nodes.dispatch.CachedSlotAccessNode.CachedSlotRead;
+import som.interpreter.nodes.dispatch.CachedSlotAccessNode.CachedSlotWrite;
+import som.interpreter.nodes.dispatch.CachedSlotAccessNode.LexicallyBoundImmutableSlotRead;
+import som.interpreter.nodes.dispatch.CachedSlotAccessNode.LexicallyBoundMutableSlotRead;
+import som.interpreter.nodes.dispatch.CachedSlotAccessNode.LexicallyBoundMutableSlotWrite;
+import som.interpreter.nodes.dispatch.DispatchGuard;
 import som.interpreter.nodes.dispatch.Dispatchable;
 import som.interpreter.nodes.literals.NilLiteralNode;
 import som.interpreter.objectstorage.ClassFactory;
@@ -38,6 +40,7 @@
 import som.vmobjects.SInvokable;
 import som.vmobjects.SInvokable.SInitializer;
 import som.vmobjects.SObject;
+import som.vmobjects.SObject.SImmutableObject;
 import som.vmobjects.SObjectWithClass;
 import som.vmobjects.SSymbol;
 
@@ -494,12 +497,17 @@ public String toString() {
 
     @Override
     public AbstractDispatchNode getDispatchNode(final Object rcvr,
-        final Object rcvrClass, final AbstractDispatchNode next) {
-      assert rcvrClass instanceof SClass;
+        final AbstractDispatchNode next) {
       if (modifier == AccessModifier.PRIVATE) {
-        return new CachedSlotAccessNode(createNode());
+        // TODO: we actually should try to remove SImmuableObject, and use a caching node
+        //       the caching node could also be more beneficial, because we can cache for all immutable fields, and if rcvr identidy check fails, we can do the load, but then still use the cached value
+        if (rcvr instanceof SImmutableObject) {
+          return new LexicallyBoundImmutableSlotRead(createNode());
+        } else {
+          return new LexicallyBoundMutableSlotRead(createNode());
+        }
       } else {
-        return new CheckedCachedSlotAccessNode(((SClass) rcvrClass).getLayoutForInstances(), createNode(), next);
+        return new CachedSlotRead(createNode(), DispatchGuard.create(rcvr), next);
       }
     }
 
@@ -562,12 +570,12 @@ public SlotMutator(final SSymbol name, final AccessModifier acccessModifier,
 
     @Override
     public AbstractDispatchNode getDispatchNode(final Object rcvr,
-        final Object rcvrClass, final AbstractDispatchNode next) {
-      assert rcvrClass instanceof SClass;
+        final AbstractDispatchNode next) {
       if (modifier == AccessModifier.PRIVATE) {
-        return new CachedSlotWriteNode(createWriteNode());
+        return new LexicallyBoundMutableSlotWrite(createWriteNode());
       } else {
-        return new CheckedCachedSlotWriteNode(((SClass) rcvrClass).getInstanceFactory(), createWriteNode(), next);
+        return new CachedSlotWrite(createWriteNode(),
+            DispatchGuard.create(rcvr), next);
       }
     }
 

src/som/interpreter/nodes/MessageSendNode.java

@@ -7,8 +7,6 @@
 import som.interpreter.nodes.dispatch.AbstractDispatchNode;
 import som.interpreter.nodes.dispatch.DispatchChain.Cost;
 import som.interpreter.nodes.dispatch.GenericDispatchNode;
-import som.interpreter.nodes.dispatch.LexicallyBoundDispatchNode;
-import som.interpreter.nodes.dispatch.SuperDispatchNode;
 import som.interpreter.nodes.dispatch.UninitializedDispatchNode;
 import som.interpreter.nodes.literals.BlockNode;
 import som.interpreter.nodes.nary.EagerBinaryPrimitiveNode;
@@ -119,7 +117,8 @@ public static GenericMessageSendNode createGeneric(final SSymbol selector,
       throw new NotYetImplementedException();
     } else {
       return new GenericMessageSendNode(selector, argumentNodes,
-          new UninitializedDispatchNode(selector, AccessModifier.PUBLIC), source);
+          UninitializedDispatchNode.createRcvrSend(selector, AccessModifier.PUBLIC),
+          source);
     }
   }
 
@@ -220,7 +219,7 @@ protected PreevaluatedExpression makeSend() {
     private GenericMessageSendNode makeOrdenarySend() {
       GenericMessageSendNode send = new GenericMessageSendNode(selector,
           argumentNodes,
-          new UninitializedDispatchNode(selector, AccessModifier.PUBLIC),
+          UninitializedDispatchNode.createRcvrSend(selector, AccessModifier.PUBLIC),
           getSourceSection());
       return replace(send);
     }
@@ -636,9 +635,9 @@ protected PreevaluatedExpression makeSpecialSend() {
       AbstractDispatchNode dispatch;
 
       if (rcvrNode.isSuperSend()) {
-        dispatch = SuperDispatchNode.create(selector, (ISuperReadNode) rcvrNode);
+        dispatch = UninitializedDispatchNode.createSuper(selector, (ISuperReadNode) rcvrNode);
       } else {
-        dispatch = new LexicallyBoundDispatchNode(selector, rcvrNode.getEnclosingMixinId());
+        dispatch = UninitializedDispatchNode.createLexicallyBound(selector, rcvrNode.getEnclosingMixinId());
       }
 
       GenericMessageSendNode node = new GenericMessageSendNode(selector,

src/som/interpreter/nodes/dispatch/AbstractCachedDnuNode.java

@@ -1,33 +1,13 @@
 package som.interpreter.nodes.dispatch;
 
-import com.oracle.truffle.api.CallTarget;
-import com.oracle.truffle.api.Truffle;
 import com.oracle.truffle.api.frame.VirtualFrame;
-import com.oracle.truffle.api.nodes.DirectCallNode;
 import com.oracle.truffle.api.nodes.Node;
 
 
-public abstract class AbstractDispatchNode extends Node implements DispatchChain {
+public abstract class AbstractDispatchNode
+    extends Node implements DispatchChain {
   public static final int INLINE_CACHE_SIZE = 6;
 
   public abstract Object executeDispatch(
       final VirtualFrame frame, final Object[] arguments);
-
-  public abstract static class AbstractCachedDispatchNode
-      extends AbstractDispatchNode {
-
-    @Child protected DirectCallNode       cachedMethod;
-    @Child protected AbstractDispatchNode nextInCache;
-
-    public AbstractCachedDispatchNode(final CallTarget methodCallTarget,
-        final AbstractDispatchNode nextInCache) {
-      this.cachedMethod = Truffle.getRuntime().createDirectCallNode(methodCallTarget);
-      this.nextInCache  = nextInCache;
-    }
-
-    @Override
-    public final int lengthOfDispatchChain() {
-      return 1 + nextInCache.lengthOfDispatchChain();
-    }
-  }
 }

src/som/interpreter/nodes/dispatch/AbstractDispatchNode.java

@@ -0,0 +1,43 @@
+package som.interpreter.nodes.dispatch;
+
+import com.oracle.truffle.api.CallTarget;
+import com.oracle.truffle.api.CompilerDirectives;
+import com.oracle.truffle.api.Truffle;
+import com.oracle.truffle.api.frame.VirtualFrame;
+import com.oracle.truffle.api.nodes.DirectCallNode;
+import com.oracle.truffle.api.nodes.InvalidAssumptionException;
+
+
+public final class CachedDispatchNode extends AbstractDispatchNode {
+
+  @Child private DirectCallNode       cachedMethod;
+  @Child private AbstractDispatchNode nextInCache;
+
+  private final DispatchGuard         guard;
+
+  public CachedDispatchNode(final CallTarget methodCallTarget,
+      final DispatchGuard guard, final AbstractDispatchNode nextInCache) {
+    this.cachedMethod = Truffle.getRuntime().createDirectCallNode(methodCallTarget);
+    this.guard        = guard;
+    this.nextInCache  = nextInCache;
+  }
+
+  @Override
+  public Object executeDispatch(final VirtualFrame frame, final Object[] arguments) {
+    try {
+      if (guard.entryMatches(arguments[0])) {
+        return cachedMethod.call(frame, arguments);
+      } else {
+        return nextInCache.executeDispatch(frame, arguments);
+      }
+    } catch (InvalidAssumptionException e) {
+      CompilerDirectives.transferToInterpreter();
+      return replace(nextInCache).executeDispatch(frame, arguments);
+    }
+  }
+
+  @Override
+  public int lengthOfDispatchChain() {
+    return 1 + nextInCache.lengthOfDispatchChain();
+  }
+}