changeset 22240:f78c72e2e0b6

Truffle/Instrumentation: clean up, better encapsulate how the application of ASTProbers is managed - Guest Language RootNode implementations no longer involved - ASTProbers are now applied only to RootNodes - Methods renamed to clarify flow of control - Allowed removal of one Accessor instance - Remove tracing overhead when there are no ASTProbers registered
author Michael Van De Vanter <michael.van.de.vanter@oracle.com>
date Tue, 22 Sep 2015 20:25:58 -0700
parents e7e3826801d6
children 14e6dfb1ef05
files truffle/com.oracle.truffle.api.vm/src/com/oracle/truffle/api/vm/SymbolInvokerImpl.java truffle/com.oracle.truffle.api.vm/src/com/oracle/truffle/api/vm/TruffleVM.java truffle/com.oracle.truffle.api/src/com/oracle/truffle/api/impl/Accessor.java truffle/com.oracle.truffle.api/src/com/oracle/truffle/api/instrument/Instrumenter.java truffle/com.oracle.truffle.api/src/com/oracle/truffle/api/nodes/Node.java truffle/com.oracle.truffle.api/src/com/oracle/truffle/api/nodes/RootNode.java truffle/com.oracle.truffle.sl/src/com/oracle/truffle/sl/nodes/SLRootNode.java
diffstat 7 files changed, 44 insertions(+), 80 deletions(-) [+]
line wrap: on
line diff
--- a/truffle/com.oracle.truffle.api.vm/src/com/oracle/truffle/api/vm/SymbolInvokerImpl.java	Tue Sep 22 15:10:25 2015 -0700
+++ b/truffle/com.oracle.truffle.api.vm/src/com/oracle/truffle/api/vm/SymbolInvokerImpl.java	Tue Sep 22 20:25:58 2015 -0700
@@ -83,11 +83,6 @@
             Object tmp = ForeignAccess.execute(foreignAccess, frame, function, args);
             return convert.convert(frame, tmp);
         }
-
-        @Override
-        public void applyInstrumentation() {
-            SymbolInvokerImpl.ACCESSOR_INTEROP.applyInstrumentation(foreignAccess);
-        }
     }
 
     private static final class ConvertNode extends Node {
@@ -131,14 +126,4 @@
             return obj;
         }
     }
-
-    static final class AccessorInterop extends Accessor {
-
-        @Override
-        protected void applyInstrumentation(Node node) {
-            super.applyInstrumentation(node);
-        }
-    }
-
-    static final AccessorInterop ACCESSOR_INTEROP = new AccessorInterop();
 }
--- a/truffle/com.oracle.truffle.api.vm/src/com/oracle/truffle/api/vm/TruffleVM.java	Tue Sep 22 15:10:25 2015 -0700
+++ b/truffle/com.oracle.truffle.api.vm/src/com/oracle/truffle/api/vm/TruffleVM.java	Tue Sep 22 20:25:58 2015 -0700
@@ -960,5 +960,6 @@
             TruffleVM vm = (TruffleVM) obj;
             vm.dispatch(event);
         }
+
     } // end of SPIAccessor
 }
--- a/truffle/com.oracle.truffle.api/src/com/oracle/truffle/api/impl/Accessor.java	Tue Sep 22 15:10:25 2015 -0700
+++ b/truffle/com.oracle.truffle.api/src/com/oracle/truffle/api/impl/Accessor.java	Tue Sep 22 20:25:58 2015 -0700
@@ -59,7 +59,6 @@
     private static Accessor API;
     private static Accessor SPI;
     private static Accessor NODES;
-    private static Accessor INTEROP;
     private static Accessor INSTRUMENT;
     private static Accessor DEBUG;
     private static final ThreadLocal<Object> CURRENT_VM = new ThreadLocal<>();
@@ -162,11 +161,6 @@
                 throw new IllegalStateException();
             }
             NODES = this;
-        } else if (this.getClass().getSimpleName().endsWith("Interop")) {
-            if (INTEROP != null) {
-                throw new IllegalStateException();
-            }
-            INTEROP = this;
         } else if (this.getClass().getSimpleName().endsWith("Instrument")) {
             if (INSTRUMENT != null) {
                 throw new IllegalStateException();
@@ -268,7 +262,7 @@
         if (known == null) {
             vm = CURRENT_VM.get();
             if (vm == null) {
-                throw new IllegalStateException();
+                throw new IllegalStateException("Accessor.findLanguage access to vm");
             }
             if (languageClass == null) {
                 return null;
@@ -284,7 +278,7 @@
         if (known == null) {
             vm = CURRENT_VM.get();
             if (vm == null) {
-                throw new IllegalStateException();
+                throw new IllegalStateException("Accessor.findLanguageImpl access to vm");
             }
         } else {
             vm = known;
@@ -297,7 +291,7 @@
         if (known == null) {
             vm = CURRENT_VM.get();
             if (vm == null) {
-                throw new IllegalStateException();
+                throw new IllegalStateException("Accessor.getInstrumenter access to vm");
             }
         } else {
             vm = known;
@@ -367,7 +361,8 @@
         return API.findLanguage(env);
     }
 
-    protected void applyInstrumentation(Node node) {
-        INSTRUMENT.applyInstrumentation(node);
+    /** Applies all registered {@linkplain ASTProber probers} to the AST. */
+    protected void probeAST(RootNode rootNode) {
+        INSTRUMENT.probeAST(rootNode);
     }
 }
--- a/truffle/com.oracle.truffle.api/src/com/oracle/truffle/api/instrument/Instrumenter.java	Tue Sep 22 15:10:25 2015 -0700
+++ b/truffle/com.oracle.truffle.api/src/com/oracle/truffle/api/instrument/Instrumenter.java	Tue Sep 22 20:25:58 2015 -0700
@@ -563,29 +563,31 @@
      * Enables instrumentation in a newly created AST by applying all registered instances of
      * {@link ASTProber}.
      */
-    private void applyInstrumentation(Node node) {
+    private void probeAST(RootNode rootNode) {
+        if (!astProbers.isEmpty()) {
 
-        String name = "<?>";
-        final Source source = findSource(node);
-        if (source != null) {
-            name = source.getShortName();
-        } else {
-            final SourceSection sourceSection = node.getEncapsulatingSourceSection();
-            if (sourceSection != null) {
-                name = sourceSection.getShortDescription();
+            String name = "<?>";
+            final Source source = findSource(rootNode);
+            if (source != null) {
+                name = source.getShortName();
+            } else {
+                final SourceSection sourceSection = rootNode.getEncapsulatingSourceSection();
+                if (sourceSection != null) {
+                    name = sourceSection.getShortDescription();
+                }
             }
+            trace("START %s", name);
+            for (ProbeListener listener : probeListeners) {
+                listener.startASTProbing(source);
+            }
+            for (ASTProber prober : astProbers) {
+                prober.probeAST(this, rootNode);  // TODO (mlvdv)
+            }
+            for (ProbeListener listener : probeListeners) {
+                listener.endASTProbing(source);
+            }
+            trace("FINISHED %s", name);
         }
-        trace("START %s", name);
-        for (ProbeListener listener : probeListeners) {
-            listener.startASTProbing(source);
-        }
-        for (ASTProber prober : astProbers) {
-            prober.probeAST(this, node);  // TODO (mlvdv)
-        }
-        for (ProbeListener listener : probeListeners) {
-            listener.endASTProbing(source);
-        }
-        trace("FINISHED %s", name);
     }
 
     static final class AccessorInstrument extends Accessor {
@@ -618,11 +620,15 @@
         }
 
         @Override
-        protected void applyInstrumentation(Node node) {
-            super.getInstrumenter(null).applyInstrumentation(node);
+        protected void probeAST(RootNode rootNode) {
+            // Normally null vm argument; can be reflectively set for testing
+            super.getInstrumenter(testVM).probeAST(rootNode);
         }
     }
 
     static final AccessorInstrument ACCESSOR = new AccessorInstrument();
 
+    // Normally null; set for testing where the Accessor hasn't been fully initialized
+    private static Object testVM = null;
+
 }
--- a/truffle/com.oracle.truffle.api/src/com/oracle/truffle/api/nodes/Node.java	Tue Sep 22 15:10:25 2015 -0700
+++ b/truffle/com.oracle.truffle.api/src/com/oracle/truffle/api/nodes/Node.java	Tue Sep 22 20:25:58 2015 -0700
@@ -41,7 +41,6 @@
 import com.oracle.truffle.api.TruffleLanguage;
 import com.oracle.truffle.api.TruffleOptions;
 import com.oracle.truffle.api.impl.Accessor;
-import com.oracle.truffle.api.instrument.WrapperNode;
 import com.oracle.truffle.api.source.SourceSection;
 import com.oracle.truffle.api.utilities.JSONHelper;
 
@@ -492,8 +491,9 @@
         return "";
     }
 
-    protected final void applyInstrumentation(Node node) {
-        ACCESSOR.applyInstrumentation(node);
+    @SuppressWarnings("static-method")
+    protected final void probeAST(RootNode rootNode) {
+        ACCESSOR.probeAST(rootNode);
     }
 
     private static final Object GIL = new Object();
@@ -527,8 +527,8 @@
         }
 
         @Override
-        protected void applyInstrumentation(Node node) {
-            super.applyInstrumentation(node);
+        protected void probeAST(RootNode rootNode) {
+            super.probeAST(rootNode);
         }
     }
 
--- a/truffle/com.oracle.truffle.api/src/com/oracle/truffle/api/nodes/RootNode.java	Tue Sep 22 15:10:25 2015 -0700
+++ b/truffle/com.oracle.truffle.api/src/com/oracle/truffle/api/nodes/RootNode.java	Tue Sep 22 20:25:58 2015 -0700
@@ -35,7 +35,6 @@
 import com.oracle.truffle.api.frame.FrameInstance;
 import com.oracle.truffle.api.frame.VirtualFrame;
 import com.oracle.truffle.api.impl.DefaultCompilerOptions;
-import com.oracle.truffle.api.instrument.ASTProber;
 import com.oracle.truffle.api.source.SourceSection;
 
 /**
@@ -161,7 +160,7 @@
      * stack) without prior knowledge of the language it has come from.
      *
      * Used for instance to determine the language of a <code>RootNode<code>:
-     *
+     * 
      * <pre>
      * <code>
      * rootNode.getExecutionContext().getLanguageShortName();
@@ -186,26 +185,13 @@
         }
     }
 
-    /**
-     * Apply to the AST all instances of {@link ASTProber} specified for the language, if any, held
-     * by this root node. This can only be done once the AST is complete, notably once all parent
-     * pointers are correctly assigned. But it also must be done before any AST cloning or
-     * execution.
-     * <p>
-     * If this is not done, then the AST will not be subject to debugging or any other
-     * instrumentation-supported tooling.
-     * <p>
-     * Implementations should ensure that instrumentation is never applied more than once to an AST,
-     * as this is not guaranteed to be error-free.
-     *
-     * @see TruffleLanguage
-     */
-    public void applyInstrumentation() {
+    public final void applyInstrumentation() {
+        super.probeAST(this);
     }
 
     /**
      * Helper method to create a root node that always returns the same value. Certain operations
-     * (expecially {@link com.oracle.truffle.api.interop inter-operability} API) require return of
+     * (especially {@link com.oracle.truffle.api.interop inter-operability} API) require return of
      * stable {@link RootNode root nodes}. To simplify creation of such nodes, here is a factory
      * method that can create {@link RootNode} that returns always the same value.
      *
--- a/truffle/com.oracle.truffle.sl/src/com/oracle/truffle/sl/nodes/SLRootNode.java	Tue Sep 22 15:10:25 2015 -0700
+++ b/truffle/com.oracle.truffle.sl/src/com/oracle/truffle/sl/nodes/SLRootNode.java	Tue Sep 22 20:25:58 2015 -0700
@@ -88,21 +88,12 @@
         this.isCloningAllowed = isCloningAllowed;
     }
 
-    public SLExpressionNode getBodyNode() {
-        return bodyNode;
-    }
-
     @Override
     public boolean isCloningAllowed() {
         return isCloningAllowed;
     }
 
     @Override
-    public void applyInstrumentation() {
-        super.applyInstrumentation(bodyNode);
-    }
-
-    @Override
     public String toString() {
         return "root " + name;
     }