# HG changeset patch
# User Michael Van De Vanter  <michael.van.de.vanter@oracle.com>
# Date 1443464868 25200
# Node ID ff9d1426d7449fb17efb3e5e9f7d0b7c0ac6e14d
# Parent  3168715cb34d99ee2f8b14f81dd0915848ad562b
Truffle/Instrumentation:  add missing notification of TagInstrument disposal; extensive Javadoc on Instrumenter

diff -r 3168715cb34d -r ff9d1426d744 truffle/com.oracle.truffle.api/src/com/oracle/truffle/api/instrument/Instrumenter.java
--- a/truffle/com.oracle.truffle.api/src/com/oracle/truffle/api/instrument/Instrumenter.java	Sun Sep 27 21:50:58 2015 -0700
+++ b/truffle/com.oracle.truffle.api/src/com/oracle/truffle/api/instrument/Instrumenter.java	Mon Sep 28 11:27:48 2015 -0700
@@ -45,7 +45,47 @@
 import com.oracle.truffle.api.source.SourceSection;
 
 /**
- * Access to instrumentation services in an execution instance.
+ * Client access to instrumentation services in a Truffle execution environment.
+ * <p>
+ * Services include:
+ * <ul>
+ * <li>A collection of {@linkplain Probe Probes}, each of which is {@linkplain #probe(Node) created}
+ * by clients in permanent association with a particular {@linkplain SourceSection source code
+ * location} in an AST executing in this environment. The Probe keeps tracks of all <em>clones</em>
+ * of the AST and ensures that any instrumentation <em>attached</em> to the Probe is put into effect
+ * in each AST clone at the {@link Node} that corresponds to the Probe's source code location.</li>
+ * <p>
+ * <li>A collection of {@linkplain ProbeListener listeners} that have registered to be notified
+ * whenever a new {@link Probe} is created in this environment and whenever a new {@link SyntaxTag}
+ * (or simply "tag") is newly added to an existing {@link Probe} in this environment.</li>
+ * <p>
+ * <li>The ability to {@linkplain #findProbesTaggedAs(SyntaxTag) enumerate} all existing
+ * {@linkplain Probe Probes} in this environment, optionally filtered to include only those to which
+ * a specific {@linkplain SyntaxTag tag} has been added.</li>
+ * <p>
+ * <li>The ability to <em>attach</em> a client-provided <em>event listener</em> to an existing
+ * {@link Probe} in this environment. The listener subsequently receives notification of execution
+ * events at the {@linkplain Node Nodes} corresponding to the Probe's {@linkplain SourceSection
+ * source code location}. The <em>attachment</em> also produces a {@link ProbeInstrument} that
+ * represents the binding, and which can be used to {@linkplain Instrument#dispose() detach} the
+ * listener from the Probe and stop event notification. A listener can be attached to any number of
+ * Probes, each time producing a new {@linkplain ProbeInstrument} that represents the binding.</li>
+ * <p>
+ * <li>The ability to <em>attach</em> a client-provided <em>event listener</em> to a specific
+ * {@linkplain SyntaxTag tag} for all {@linkplain Probe Probes} in the environment. A maximum of
+ * <em>one</em> listener may be attached to receive notification of "<em>before</em>" execution
+ * events (i.e. the flow of execution is just about to enter a {@link Node}), and a maximum of
+ * <em>one</em> listener may be attached to receive notification of "<em>after</em>" execution
+ * events. The <em>attachment</em> also produces a {@link TagInstrument} that represents the
+ * binding, and which can be used to {@linkplain Instrument#dispose() detach} the listener from the
+ * Probes and stop event notification. This mechanism is designed for much lower runtime overhead
+ * than other ways to accomplish the same thing, e.g. by attaching one listener individually to
+ * every Probe with the desired tag.</li>
+ * <p>
+ * <li>A collection of {@linkplain Tool Tools}, possibly client-provided, that can be
+ * {@linkplain #install(Tool) installed} for data collection, possibly providing their own services
+ * with the resulting information.</li>
+ * </ul>
  */
 public final class Instrumenter {
 
@@ -75,7 +115,7 @@
     }
 
     /**
-     * {@linkplain ProbeInstrument Instrumentation}-based collectors of data during Guest Language
+     * {@linkplain Instrumenter Instrumentation}-based collectors of data during Guest Language
      * program execution.
      * <p>
      * Tools share a common <em>life cycle</em>:
@@ -265,7 +305,7 @@
      * It is a runtime error to attempt Probing an AST node with no parent.
      *
      * @return a (possibly newly created) {@link Probe} associated with this node.
-     * @throws ProbeException (unchecked) when a probe cannot be created, leaving the AST unchanged
+     * @throws ProbeException (unchecked) when a Probe cannot be created, leaving the AST unchanged
      */
     @SuppressWarnings("rawtypes")
     public Probe probe(Node node) {
@@ -295,7 +335,7 @@
             throw new ProbeException(ProbeFailure.Reason.NOT_INSTRUMENTABLE, parent, node, null);
         }
 
-        // Create a new wrapper/probe with this node as its child.
+        // Create a new wrapper/Probe with this node as its child.
         final WrapperNode wrapper = createWrapperNode(node);
 
         if (wrapper == null || !(wrapper instanceof Node)) {
@@ -382,7 +422,7 @@
      * {@linkplain EventHandlerNode execution events} taking place at the Probe's AST location to
      * the listener.
      *
-     * @param probe source of execution events
+     * @param probe source of AST execution events
      * @param listener receiver of execution events
      * @param instrumentInfo optional documentation about the Instrument
      * @return a handle for access to the binding
@@ -401,7 +441,7 @@
      * {@linkplain EventHandlerNode execution events} taking place at the Probe's AST location to
      * the listener.
      *
-     * @param probe source of execution events
+     * @param probe source of AST execution events
      * @param listener receiver of execution events
      * @param instrumentInfo optional documentation about the Instrument
      * @return a handle for access to the binding
@@ -426,7 +466,7 @@
      * Any {@link RuntimeException} thrown by execution of the fragment is caught by the framework
      * and reported to the listener; there is no other notification.
      *
-     * @param probe probe source of execution events
+     * @param probe source of AST execution events
      * @param listener optional client callback for results/failure notification
      * @param rootFactory provider of AST fragments on behalf of the client
      * @param requiredResultType optional requirement, any non-assignable result is reported to the
@@ -461,13 +501,8 @@
         if (beforeTagInstrument != null) {
             throw new IllegalStateException("Only one 'before' TagInstrument at a time");
         }
-        this.beforeTagInstrument = new TagInstrument.BeforeTagInstrument(tag, listener, instrumentInfo);
-        for (WeakReference<Probe> ref : probes) {
-            final Probe probe = ref.get();
-            if (probe != null) {
-                probe.notifyTagInstrumentsChanged();
-            }
-        }
+        this.beforeTagInstrument = new TagInstrument.BeforeTagInstrument(this, tag, listener, instrumentInfo);
+        notifyTagInstrumentChange();
         return beforeTagInstrument;
     }
 
@@ -491,13 +526,8 @@
         if (afterTagInstrument != null) {
             throw new IllegalStateException("Only one 'afater' TagInstrument at a time");
         }
-        this.afterTagInstrument = new TagInstrument.AfterTagInstrument(tag, listener, instrumentInfo);
-        for (WeakReference<Probe> ref : probes) {
-            final Probe probe = ref.get();
-            if (probe != null) {
-                probe.notifyTagInstrumentsChanged();
-            }
-        }
+        this.afterTagInstrument = new TagInstrument.AfterTagInstrument(this, tag, listener, instrumentInfo);
+        notifyTagInstrumentChange();
         return afterTagInstrument;
     }
 
@@ -537,6 +567,25 @@
         return afterTagInstrument;
     }
 
+    void disposeBeforeTagInstrument() {
+        beforeTagInstrument = null;
+        notifyTagInstrumentChange();
+    }
+
+    void disposeAfterTagInstrument() {
+        afterTagInstrument = null;
+        notifyTagInstrumentChange();
+    }
+
+    private void notifyTagInstrumentChange() {
+        for (WeakReference<Probe> ref : probes) {
+            final Probe probe = ref.get();
+            if (probe != null) {
+                probe.notifyTagInstrumentsChanged();
+            }
+        }
+    }
+
     // TODO (mlvdv) build this in as a VM event?
     /**
      * Enables instrumentation in a newly created AST by applying all registered instances of
diff -r 3168715cb34d -r ff9d1426d744 truffle/com.oracle.truffle.api/src/com/oracle/truffle/api/instrument/TagInstrument.java
--- a/truffle/com.oracle.truffle.api/src/com/oracle/truffle/api/instrument/TagInstrument.java	Sun Sep 27 21:50:58 2015 -0700
+++ b/truffle/com.oracle.truffle.api/src/com/oracle/truffle/api/instrument/TagInstrument.java	Mon Sep 28 11:27:48 2015 -0700
@@ -45,14 +45,17 @@
      */
     @SuppressWarnings("unused") private final String instrumentInfo;
 
+    protected Instrumenter instrumenter;
+
     /**
      * Has this instrument been disposed? stays true once set.
      */
-    private boolean isDisposed = false;
+    protected boolean isDisposed = false;
 
     private SyntaxTag tag = null;
 
-    protected TagInstrument(SyntaxTag tag, String instrumentInfo) {
+    protected TagInstrument(Instrumenter instrumenter, SyntaxTag tag, String instrumentInfo) {
+        this.instrumenter = instrumenter;
         this.tag = tag;
         this.instrumentInfo = instrumentInfo;
     }
@@ -62,8 +65,7 @@
         if (isDisposed) {
             throw new IllegalStateException("Attempt to dispose an already disposed Instrumennt");
         }
-
-        // TODO (mlvdv)
+        instrumenter.disposeAfterTagInstrument();
         this.isDisposed = true;
     }
 
@@ -80,27 +82,45 @@
 
         private final StandardBeforeInstrumentListener listener;
 
-        BeforeTagInstrument(SyntaxTag tag, StandardBeforeInstrumentListener listener, String instrumentInfo) {
-            super(tag, instrumentInfo);
+        BeforeTagInstrument(Instrumenter instrumenter, SyntaxTag tag, StandardBeforeInstrumentListener listener, String instrumentInfo) {
+            super(instrumenter, tag, instrumentInfo);
             this.listener = listener;
         }
 
         StandardBeforeInstrumentListener getListener() {
             return listener;
         }
+
+        @Override
+        public void dispose() throws IllegalStateException {
+            if (isDisposed) {
+                throw new IllegalStateException("Disposed Instrument can not be disposed again");
+            }
+            instrumenter.disposeBeforeTagInstrument();
+            this.isDisposed = true;
+        }
     }
 
     static final class AfterTagInstrument extends TagInstrument {
 
         private final StandardAfterInstrumentListener listener;
 
-        AfterTagInstrument(SyntaxTag tag, StandardAfterInstrumentListener listener, String instrumentInfo) {
-            super(tag, instrumentInfo);
+        AfterTagInstrument(Instrumenter instrumenter, SyntaxTag tag, StandardAfterInstrumentListener listener, String instrumentInfo) {
+            super(instrumenter, tag, instrumentInfo);
             this.listener = listener;
         }
 
         StandardAfterInstrumentListener getListener() {
             return listener;
         }
+
+        @Override
+        public void dispose() throws IllegalStateException {
+            if (isDisposed) {
+                throw new IllegalStateException("Disposed Instrument can not be disposed again");
+            }
+            instrumenter.disposeAfterTagInstrument();
+            this.isDisposed = true;
+        }
     }
 }