changeset 16857:7bfbad29d331

Truffle/Instrumentation: Javadoc cleanups and minor corrections.
author Michael Van De Vanter <michael.van.de.vanter@oracle.com>
date Mon, 18 Aug 2014 21:03:41 -0700
parents 7833417c8172
children a8af2abc2039
files graal/com.oracle.truffle.api/src/com/oracle/truffle/api/ExecutionContext.java graal/com.oracle.truffle.api/src/com/oracle/truffle/api/instrument/Probe.java graal/com.oracle.truffle.api/src/com/oracle/truffle/api/instrument/impl/InstrumentationNode.java graal/com.oracle.truffle.api/src/com/oracle/truffle/api/instrument/impl/LineLocationToProbeCollectionMap.java graal/com.oracle.truffle.api/src/com/oracle/truffle/api/instrument/impl/ProbeManager.java
diffstat 5 files changed, 97 insertions(+), 121 deletions(-) [+]
line wrap: on
line diff
--- a/graal/com.oracle.truffle.api/src/com/oracle/truffle/api/ExecutionContext.java	Mon Aug 18 14:36:12 2014 -0700
+++ b/graal/com.oracle.truffle.api/src/com/oracle/truffle/api/ExecutionContext.java	Mon Aug 18 21:03:41 2014 -0700
@@ -102,10 +102,10 @@
     }
 
     /**
-     * Return a newly created {@link Probe} uniquely associated with a particular source section. A
-     * newly created probe carries no tags.
+     * Return a newly created, untagged, {@link Probe} associated with a particular source section,
+     * with no requirement that the association be unique.
      *
-     * @return a probe uniquely associated with an extent of guest language source code.
+     * @return a probe associated with an extent of guest language source code.
      */
     public final Probe createProbe(SourceSection source) {
         return probeManager.createProbe(source);
--- a/graal/com.oracle.truffle.api/src/com/oracle/truffle/api/instrument/Probe.java	Mon Aug 18 14:36:12 2014 -0700
+++ b/graal/com.oracle.truffle.api/src/com/oracle/truffle/api/instrument/Probe.java	Mon Aug 18 21:03:41 2014 -0700
@@ -45,9 +45,6 @@
  * attached instruments seldom changes. The assumption is invalidated when instruments are added or
  * removed, but some instruments may change their internal state in such a way that the assumption
  * should also be invalidated.
- * <p>
- * <strong>Disclaimer:</strong> experimental interface under development. In particular, the
- * <em>notify</em> methods must be migrated to another interface.
  *
  * @see Instrument
  * @see Wrapper
@@ -55,27 +52,34 @@
 public interface Probe extends ExecutionEvents, SyntaxTagged {
 
     /**
-     * Get the {@link SourceSection} associated with this probe. This is no longer uniquely
-     * associated.
+     * Get the {@link SourceSection} in some Truffle AST associated with this probe.
      *
      * @return The source associated with this probe.
      */
     SourceSection getSourceLocation();
 
     /**
-     * Mark this probe as being associated with an AST node in some category useful for debugging
-     * and other tools.
+     * Mark this probe as belonging to some tool-related category that can be used to guide tool
+     * behavior at an associated AST node. For example, a debugger might add the tag
+     * {@link StandardSyntaxTag#STATEMENT} as a way of configuring where execution should stop when
+     * stepping.
      */
     void tagAs(SyntaxTag tag);
 
     /**
-     * Adds an instrument to this probe.
+     * Adds an {@link Instrument} to this probe's collection. No check is made to see if the same
+     * instrument has already been added.
+     *
+     * @param newInstrument The instrument to add to this probe.
      */
     void addInstrument(Instrument newInstrument);
 
     /**
-     * Removes an instrument from this probe.
+     * Removes the given instrument from the probe's collection.
+     *
+     * @param oldInstrument The instrument to remove from this probe.
+     * @throws RuntimeException if no matching instrument has been attached.
      */
-    void removeInstrument(Instrument oldInstrument);
+    void removeInstrument(Instrument oldInstrument) throws RuntimeException;
 
 }
--- a/graal/com.oracle.truffle.api/src/com/oracle/truffle/api/instrument/impl/InstrumentationNode.java	Mon Aug 18 14:36:12 2014 -0700
+++ b/graal/com.oracle.truffle.api/src/com/oracle/truffle/api/instrument/impl/InstrumentationNode.java	Mon Aug 18 21:03:41 2014 -0700
@@ -34,11 +34,9 @@
 import com.oracle.truffle.api.source.*;
 
 /**
- * Abstract implementation of Truffle {@link Node} to be used for AST probes and instruments.
- * Instruments are represented as a chain using the next field. Each instrument chain is connected
- * to a single probe.
- * <p>
- * Coordinates propagation of Truffle AST {@link ExecutionEvents}.
+ * Abstract implementation of Truffle {@link Node}s used as AST {@link Probe}s and
+ * {@link Instrument}s. A {@link Probe} manages its attached {@link Instrument}s by appending them
+ * to a chain through which {@link ExecutionEvents} are propagated.
  */
 public abstract class InstrumentationNode extends Node implements ExecutionEvents {
 
@@ -112,9 +110,9 @@
     }
 
     /**
-     * Inform the instrument that the AST node this is attached to has just been entered. This will
-     * continue to call {@link #internalEnter(Node, VirtualFrame)} to inform all instruments in the
-     * chain.
+     * Informs the instrument that execution is just about to enter an AST node with which this
+     * instrumentation node is associated. This will continue to call
+     * {@link #internalEnter(Node, VirtualFrame)} to inform all instruments in the chain.
      *
      * @param astNode The {@link Node} that was entered
      * @param frame The {@link VirtualFrame} at the time of entry
@@ -127,9 +125,10 @@
     }
 
     /**
-     * Inform the instrument that the AST node this is attached to is about to be left. This will
-     * continue to call {@link #internalEnter(Node, VirtualFrame)} to inform all instruments in the
-     * chain. In this case, there is no return value.
+     * Informs the instrument that execution has just returned from an AST node with which this
+     * instrumentation node is associated. This will continue to call
+     * {@link #internalEnter(Node, VirtualFrame)} to inform all instruments in the chain. In this
+     * case, there is no return value.
      *
      * @param astNode The {@link Node} that was entered
      * @param frame The {@link VirtualFrame} at the time of exit
@@ -142,9 +141,10 @@
     }
 
     /**
-     * Inform the instrument that the AST node this is attached to has just been left. This will
-     * continue to call {@link #internalLeave(Node, VirtualFrame, boolean)} to inform all
-     * instruments in the chain. In this case, a boolean value was returned.
+     * Informs the instrument that execution has just returned from an AST node with which this
+     * instrumentation node is associated. This will continue to call
+     * {@link #internalLeave(Node, VirtualFrame, boolean)} to inform all instruments in the chain.
+     * In this case, a boolean value was returned.
      *
      * @param astNode The {@link Node} that was entered
      * @param frame The {@link VirtualFrame} at the time of exit
@@ -158,9 +158,10 @@
     }
 
     /**
-     * Inform the instrument that the AST node this is attached to is about to be left. This will
-     * continue to call {@link #internalLeave(Node, VirtualFrame, byte)} to inform all instruments
-     * in the chain. In this case, a byte value was returned.
+     * Informs the instrument that execution has just returned from an AST node with which this
+     * instrumentation node is associated. This will continue to call
+     * {@link #internalLeave(Node, VirtualFrame, byte)} to inform all instruments in the chain. In
+     * this case, a byte value was returned.
      *
      * @param astNode The {@link Node} that was entered
      * @param frame The {@link VirtualFrame} at the time of exit
@@ -174,9 +175,10 @@
     }
 
     /**
-     * Inform the instrument that the AST node this is attached to is about to be left. This will
-     * continue to call {@link #internalLeave(Node, VirtualFrame, short)} to inform all instruments
-     * in the chain. In this case, a short value was returned.
+     * Informs the instrument that execution has just returned from an AST node with which this
+     * instrumentation node is associated. This will continue to call
+     * {@link #internalLeave(Node, VirtualFrame, short)} to inform all instruments in the chain. In
+     * this case, a short value was returned.
      *
      * @param astNode The {@link Node} that was entered
      * @param frame The {@link VirtualFrame} at the time of exit
@@ -190,9 +192,10 @@
     }
 
     /**
-     * Inform the instrument that the AST node this is attached to is about to be left. This will
-     * continue to call {@link #internalLeave(Node, VirtualFrame, int)} to inform all instruments in
-     * the chain. In this case, a int value was returned.
+     * Informs the instrument that execution has just returned from an AST node with which this
+     * instrumentation node is associated. This will continue to call
+     * {@link #internalLeave(Node, VirtualFrame, int)} to inform all instruments in the chain. In
+     * this case, a int value was returned.
      *
      * @param astNode The {@link Node} that was entered
      * @param frame The {@link VirtualFrame} at the time of exit
@@ -206,9 +209,10 @@
     }
 
     /**
-     * Inform the instrument that the AST node this is attached to is about to be left. This will
-     * continue to call {@link #internalLeave(Node, VirtualFrame, long)} to inform all instruments
-     * in the chain. In this case, a long value was returned.
+     * Informs the instrument that execution has just returned from an AST node with which this
+     * instrumentation node is associated. This will continue to call
+     * {@link #internalLeave(Node, VirtualFrame, long)} to inform all instruments in the chain. In
+     * this case, a long value was returned.
      *
      * @param astNode The {@link Node} that was entered
      * @param frame The {@link VirtualFrame} at the time of exit
@@ -222,9 +226,10 @@
     }
 
     /**
-     * Inform the instrument that the AST node this is attached to is about to be left. This will
-     * continue to call {@link #internalLeave(Node, VirtualFrame, char)} to inform all instruments
-     * in the chain. In this case, a char value was returned.
+     * Informs the instrument that execution has just returned from an AST node with which this
+     * instrumentation node is associated. This will continue to call
+     * {@link #internalLeave(Node, VirtualFrame, char)} to inform all instruments in the chain. In
+     * this case, a char value was returned.
      *
      * @param astNode The {@link Node} that was entered
      * @param frame The {@link VirtualFrame} at the time of exit
@@ -238,9 +243,10 @@
     }
 
     /**
-     * Inform the instrument that the AST node this is attached to is about to be left. This will
-     * continue to call {@link #internalLeave(Node, VirtualFrame, float)} to inform all instruments
-     * in the chain. In this case, a float value was returned.
+     * Informs the instrument that execution has just returned from an AST node with which this
+     * instrumentation node is associated. This will continue to call
+     * {@link #internalLeave(Node, VirtualFrame, float)} to inform all instruments in the chain. In
+     * this case, a float value was returned.
      *
      * @param astNode The {@link Node} that was entered
      * @param frame The {@link VirtualFrame} at the time of exit
@@ -254,9 +260,10 @@
     }
 
     /**
-     * Inform the instrument that the AST node this is attached to is about to be left. This will
-     * continue to call {@link #internalLeave(Node, VirtualFrame, double)} to inform all instruments
-     * in the chain. In this case, a double value was returned.
+     * Informs the instrument that execution has just returned from an AST node with which this
+     * instrumentation node is associated. This will continue to call
+     * {@link #internalLeave(Node, VirtualFrame, double)} to inform all instruments in the chain. In
+     * this case, a double value was returned.
      *
      * @param astNode The {@link Node} that was entered
      * @param frame The {@link VirtualFrame} at the time of exit
@@ -270,9 +277,10 @@
     }
 
     /**
-     * Inform the instrument that the AST node this is attached to is about to be left. This will
-     * continue to call {@link #internalLeave(Node, VirtualFrame, Object)} to inform all instruments
-     * in the chain. In this case, an Object was returned.
+     * Informs the instrument that execution has just returned from an AST node with which this
+     * instrumentation node is associated. This will continue to call
+     * {@link #internalLeave(Node, VirtualFrame, Object)} to inform all instruments in the chain. In
+     * this case, an Object was returned.
      *
      * @param astNode The {@link Node} that was entered
      * @param frame The {@link VirtualFrame} at the time of exit
@@ -286,10 +294,10 @@
     }
 
     /**
-     * Inform the instrument that the AST node this is attached to is about to be left. This will
-     * continue to call {@link #internalLeaveExceptional(Node, VirtualFrame, Exception)} to inform
-     * all instruments in the chain. In this case, a exception (sometimes containing a value) was
-     * returned.
+     * Informs the instrument that execution has just returned from an AST node with which this
+     * instrumentation node is associated. This will continue to call
+     * {@link #internalLeaveExceptional(Node, VirtualFrame, Exception)} to inform all instruments in
+     * the chain. In this case, a exception (sometimes containing a value) was thrown.
      *
      * @param astNode The {@link Node} that was entered
      * @param frame The {@link VirtualFrame} at the time of exit
@@ -304,18 +312,17 @@
 
     /**
      * Holder of a chain of {@linkplain InstrumentationNode instruments}: manages the
-     * {@link Assumption} that none of the instruments have changed since last checked.
+     * {@link Assumption} that no {@link Instrument}s have been added or removed and that none of
+     * the attached instruments have changed state in a way that would require deopt.
      * <p>
      * An instance is intended to be shared by every clone of the AST node with which it is
      * originally attached, so it holds no parent pointer.
      * <p>
-     * Each probe is associated with a {@link SourceSection} which was originally intended to be a
-     * unique id for the probe, but this is no longer true. Instead, it is the responsibility of the
-     * tool using to probes to track which probes they are interested in. One approach is to use the
-     * callback functionality in {@link ProbeManager}.
+     * Each probe is associated with a {@link SourceSection}, not necessarily uniquely, although
+     * such a policy could be enforced for some uses.
      * <p>
-     * May be categorized by one or more {@linkplain SyntaxTag tags}, signifying information useful
-     * for instrumentation about its AST location(s).
+     * Each {@link Probe} be categorized by one or more {@linkplain SyntaxTag tags}, signifying
+     * information useful for instrumentation about its AST location(s).
      */
     static final class ProbeImpl extends InstrumentationNode implements Probe {
 
@@ -332,7 +339,9 @@
         private final ArrayList<SyntaxTag> tags = new ArrayList<>();
 
         /**
-         * The wrapper that this probe belongs to.
+         * The region of source code associated with this probe. Note that this is distinct from
+         * {@link Node#getSourceSection()}, which is {@code null} for all instances of
+         * {@link InstrumentationNode} since they have no corresponding source of their own.
          */
         private final SourceSection source;
 
@@ -350,19 +359,10 @@
             this.next = null;
         }
 
-        /**
-         * Returns the {@link SourceSection} associated with this probe.
-         */
         public SourceSection getSourceLocation() {
             return source;
         }
 
-        /**
-         * Tags this probe with the given {@link SyntaxTag}. If the tag already exists, the tag is
-         * not added.
-         *
-         * @param tag The tag to add to this probe.
-         */
         @SlowPath
         public void tagAs(SyntaxTag tag) {
             assert tag != null;
@@ -372,22 +372,11 @@
             }
         }
 
-        /**
-         * Checks if this probe has been tagged with the given tag.
-         *
-         * @param tag The {@link SyntaxTag} to check for.
-         * @return True if this probe has the given tag, false otherwise.
-         */
         public boolean isTaggedAs(SyntaxTag tag) {
             assert tag != null;
             return tags.contains(tag);
         }
 
-        /**
-         * Returns an iterable collection of all syntax tags on this probe.
-         *
-         * @return A collection of {@link SyntaxTag}s.
-         */
         public Iterable<SyntaxTag> getSyntaxTags() {
             return tags;
         }
--- a/graal/com.oracle.truffle.api/src/com/oracle/truffle/api/instrument/impl/LineLocationToProbeCollectionMap.java	Mon Aug 18 14:36:12 2014 -0700
+++ b/graal/com.oracle.truffle.api/src/com/oracle/truffle/api/instrument/impl/LineLocationToProbeCollectionMap.java	Mon Aug 18 21:03:41 2014 -0700
@@ -30,9 +30,8 @@
 import com.oracle.truffle.api.source.*;
 
 /**
- * This class maintains a mapping from {@link LineLocation} to a collection of {@link Probe}s to be
- * used by debugging tools.
- *
+ * A mapping from {@link LineLocation} (a line number in a specific piece of {@link Source} code) to
+ * a collection of {@link Probe}s whose associated {@link SourceSection} starts on that line.
  */
 public class LineLocationToProbeCollectionMap implements ProbeListener {
     /**
@@ -43,24 +42,18 @@
     public LineLocationToProbeCollectionMap() {
     }
 
-    /**
-     * Adds the current wrapper's child's line location and probe to this map.
-     */
     public void newProbeInserted(SourceSection source, Probe probe) {
         final LineLocation line = source.getLineLocation();
         this.addProbeToLine(line, probe);
     }
 
-    /**
-     * Does nothing.
-     */
     public void probeTaggedAs(Probe probe, SyntaxTag tag) {
-
+        // This map ignores tags
     }
 
     /**
      * Returns the {@link Probe}, if any, associated with source that starts on a specified line; if
-     * there are more than one, return the one with the first character location.
+     * there are more than one, return the one with the first starting character location.
      */
     public Probe findLineProbe(LineLocation lineLocation) {
         Probe probe = null;
--- a/graal/com.oracle.truffle.api/src/com/oracle/truffle/api/instrument/impl/ProbeManager.java	Mon Aug 18 14:36:12 2014 -0700
+++ b/graal/com.oracle.truffle.api/src/com/oracle/truffle/api/instrument/impl/ProbeManager.java	Mon Aug 18 21:03:41 2014 -0700
@@ -36,19 +36,12 @@
  */
 public final class ProbeManager {
 
-    /**
-     * The collection of {@link ProbeListener}s to call.
-     */
     private final List<ProbeListener> probeListeners = new ArrayList<>();
 
+    private final List<ProbeImpl> allProbes = new ArrayList<>();
+
     /**
-     * The collection of all probes added.
-     */
-    private final List<ProbeImpl> probeList = new ArrayList<>();
-
-    /**
-     * The callback to be triggered by the {@link #tagTrap}.
-     *
+     * Called when a {@link #tagTrap} is activated in a Probe.
      */
     private final ProbeCallback probeCallback;
 
@@ -99,8 +92,8 @@
     }
 
     /**
-     * Creates a {@link Probe} for the given {@link SourceSection} and informs all
-     * {@link ProbeListener}s stored in this manager of its creation.
+     * Creates a new {@link Probe} associated with a {@link SourceSection} of code corresponding to
+     * a Trufle AST node.
      *
      * @param source The source section to associate with this probe.
      * @return The probe that was created.
@@ -109,7 +102,7 @@
         assert source != null;
 
         ProbeImpl probe = InstrumentationNode.createProbe(source, probeCallback);
-        probeList.add(probe);
+        allProbes.add(probe);
 
         for (ProbeListener listener : probeListeners) {
             listener.newProbeInserted(source, probe);
@@ -119,15 +112,12 @@
     }
 
     /**
-     * Returns a collection of {@link Probe}s created by this manager that have the given
-     * {@link SyntaxTag}.
-     *
-     * @param tag The tag to search for.
-     * @return An iterable collection of probes containing the given tag.
+     * Returns the subset of all {@link Probe}s holding a particular {@link SyntaxTag}, or the whole
+     * collection if the specified tag is {@code null}.
      */
     public Collection<Probe> findProbesTaggedAs(SyntaxTag tag) {
         final List<Probe> probes = new ArrayList<>();
-        for (Probe probe : probeList) {
+        for (Probe probe : allProbes) {
             if (tag == null || probe.isTaggedAs(tag)) {
                 probes.add(probe);
             }
@@ -137,13 +127,12 @@
 
     /**
      * Calls {@link ProbeImpl#setTrap(SyntaxTagTrap)} for all probes with the given
-     * {@link SyntaxTag} . There can only be one tag trap set at a time. An
-     * {@link IllegalStateException} is thrown if this is called and a tag trap has already been
-     * set.
+     * {@link SyntaxTag} . There can only be one tag trap set at a time.
      *
      * @param tagTrap The {@link SyntaxTagTrap} to set.
+     * @throws IllegalStateException if a trap is currently set.
      */
-    public void setTagTrap(SyntaxTagTrap tagTrap) {
+    public void setTagTrap(SyntaxTagTrap tagTrap) throws IllegalStateException {
         assert tagTrap != null;
         if (this.tagTrap != null) {
             throw new IllegalStateException("trap already set");
@@ -151,7 +140,7 @@
         this.tagTrap = tagTrap;
 
         SyntaxTag tag = tagTrap.getTag();
-        for (ProbeImpl probe : probeList) {
+        for (ProbeImpl probe : allProbes) {
             if (probe.isTaggedAs(tag)) {
                 probe.setTrap(tagTrap);
             }
@@ -159,14 +148,15 @@
     }
 
     /**
-     * Clears the current {@link SyntaxTagTrap}. If no trap has been set, an
-     * {@link IllegalStateException} is thrown.
+     * Clears the current {@link SyntaxTagTrap}.
+     *
+     * @throws IllegalStateException if no trap is currently set.
      */
     public void clearTagTrap() {
         if (this.tagTrap == null) {
             throw new IllegalStateException("no trap set");
         }
-        for (ProbeImpl probe : probeList) {
+        for (ProbeImpl probe : allProbes) {
             if (probe.isTaggedAs(tagTrap.getTag())) {
                 probe.setTrap(null);
             }