# HG changeset patch
# User Michael Van De Vanter  <michael.van.de.vanter@oracle.com>
# Date 1423615459 28800
# Node ID a2ff253c458f1f156f7e2221cc31b6bd9e91527f
# Parent  128586040207db904645dcfbef72501c750a3a7b
Truffle/Instrumentation:  code cleanups in tools CoverageTracker and NodeExecCounter, especially for tutorial value

diff -r 128586040207 -r a2ff253c458f graal/com.oracle.truffle.api/src/com/oracle/truffle/api/tools/CoverageTracker.java
--- a/graal/com.oracle.truffle.api/src/com/oracle/truffle/api/tools/CoverageTracker.java	Tue Feb 10 16:44:11 2015 -0800
+++ b/graal/com.oracle.truffle.api/src/com/oracle/truffle/api/tools/CoverageTracker.java	Tue Feb 10 16:44:19 2015 -0800
@@ -29,7 +29,6 @@
 import java.util.Map.Entry;
 import java.util.concurrent.atomic.*;
 
-import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
 import com.oracle.truffle.api.frame.*;
 import com.oracle.truffle.api.instrument.*;
 import com.oracle.truffle.api.instrument.impl.*;
@@ -52,7 +51,7 @@
  * <p>
  * <ul>
  * <li>"Execution call" on a node is is defined as invocation of a node method that is instrumented
- * to produce the event {@link TruffleEventReceiver#enter(Node, VirtualFrame)};</li>
+ * to produce the event {@link TruffleEventListener#enter(Node, VirtualFrame)};</li>
  * <li>Execution calls are tabulated only at <em>instrumented</em> nodes, i.e. those for which
  * {@linkplain Node#isInstrumentable() isInstrumentable() == true};</li>
  * <li>Execution calls are tabulated only at nodes present in the AST when originally created;
@@ -75,13 +74,13 @@
 public final class CoverageTracker extends InstrumentationTool {
 
     /** Counting data. */
-    private final Map<LineLocation, CoverageCounter> counters = new HashMap<>();
+    private final Map<LineLocation, CoverageRecord> coverageMap = new HashMap<>();
 
-    /** For disposal. */
+    /** Needed for disposal. */
     private final List<Instrument> instruments = new ArrayList<>();
 
     /**
-     * Counting is restricted to nodes holding this tag.
+     * Coverage counting is restricted to nodes holding this tag.
      */
     private final SyntaxTag countingTag;
 
@@ -112,7 +111,7 @@
 
     @Override
     protected void internalReset() {
-        counters.clear();
+        coverageMap.clear();
     }
 
     @Override
@@ -142,22 +141,21 @@
          * every line associated with an appropriately tagged AST node; iterable in order of source
          * name, then line number.
          */
-        final TreeSet<Entry<LineLocation, CoverageCounter>> entries = new TreeSet<>(new LineLocationEntryComparator());
+        final TreeSet<Entry<LineLocation, CoverageRecord>> entries = new TreeSet<>(new LineLocationEntryComparator());
 
-        final Map<Source, Long[]> results = new HashMap<>();
-
-        for (Entry<LineLocation, CoverageCounter> entry : counters.entrySet()) {
+        for (Entry<LineLocation, CoverageRecord> entry : coverageMap.entrySet()) {
             entries.add(entry);
         }
+        final Map<Source, Long[]> result = new HashMap<>();
         Source curSource = null;
         Long[] curLineTable = null;
-        for (Entry<LineLocation, CoverageCounter> entry : entries) {
+        for (Entry<LineLocation, CoverageRecord> entry : entries) {
             final LineLocation key = entry.getKey();
             final Source source = key.getSource();
             final int lineNo = key.getLineNumber();
             if (source != curSource) {
                 if (curSource != null) {
-                    results.put(curSource, curLineTable);
+                    result.put(curSource, curLineTable);
                 }
                 curSource = source;
                 curLineTable = new Long[source.getLineCount()];
@@ -165,9 +163,9 @@
             curLineTable[lineNo - 1] = entry.getValue().count.longValue();
         }
         if (curSource != null) {
-            results.put(curSource, curLineTable);
+            result.put(curSource, curLineTable);
         }
-        return results;
+        return result;
     }
 
     /**
@@ -183,14 +181,14 @@
          * every line associated with an appropriately tagged AST node; iterable in order of source
          * name, then line number.
          */
-        final TreeSet<Entry<LineLocation, CoverageCounter>> entries = new TreeSet<>(new LineLocationEntryComparator());
+        final TreeSet<Entry<LineLocation, CoverageRecord>> entries = new TreeSet<>(new LineLocationEntryComparator());
 
-        for (Entry<LineLocation, CoverageCounter> entry : counters.entrySet()) {
+        for (Entry<LineLocation, CoverageRecord> entry : coverageMap.entrySet()) {
             entries.add(entry);
         }
         Source curSource = null;
         int curLineNo = 1;
-        for (Entry<LineLocation, CoverageCounter> entry : entries) {
+        for (Entry<LineLocation, CoverageRecord> entry : entries) {
             final LineLocation key = entry.getKey();
             final Source source = key.getSource();
             final int lineNo = key.getLineNumber();
@@ -228,9 +226,9 @@
     }
 
     /**
-     * A receiver for events at each instrumented AST location. This receiver counts
-     * "execution calls" to the instrumented node and is <em>stateful</em>. State in receivers must
-     * be considered carefully since ASTs, along with all instrumentation (including event receivers
+     * A listener for events at each instrumented AST location. This listener counts
+     * "execution calls" to the instrumented node and is <em>stateful</em>. State in listeners must
+     * be considered carefully since ASTs, along with all instrumentation (including event listener
      * such as this) are routinely cloned by the Truffle runtime. AST cloning is <em>shallow</em>
      * (for non- {@link Child} nodes), so in this case the actual count <em>is shared</em> among all
      * the clones; the count is also held in a table indexed by source line.
@@ -238,7 +236,7 @@
      * In contrast, a primitive field would <em>not</em> be shared among clones and resulting counts
      * would not be accurate.
      */
-    private final class CoverageEventReceiver extends DefaultEventReceiver {
+    private final class CoverageEventListener extends DefaultEventListener {
 
         /**
          * Shared by all clones of the associated instrument and by the table of counters for the
@@ -246,12 +244,11 @@
          */
         private final AtomicLong count;
 
-        CoverageEventReceiver(AtomicLong count) {
+        CoverageEventListener(AtomicLong count) {
             this.count = count;
         }
 
         @Override
-        @TruffleBoundary
         public void enter(Node node, VirtualFrame frame) {
             if (isEnabled()) {
                 count.getAndIncrement();
@@ -259,9 +256,9 @@
         }
     }
 
-    private static final class LineLocationEntryComparator implements Comparator<Entry<LineLocation, CoverageCounter>> {
+    private static final class LineLocationEntryComparator implements Comparator<Entry<LineLocation, CoverageRecord>> {
 
-        public int compare(Entry<LineLocation, CoverageCounter> e1, Entry<LineLocation, CoverageCounter> e2) {
+        public int compare(Entry<LineLocation, CoverageRecord> e1, Entry<LineLocation, CoverageRecord> e2) {
             return LineLocation.COMPARATOR.compare(e1.getKey(), e2.getKey());
         }
     }
@@ -279,35 +276,37 @@
                 if (srcSection == null) {
                     // TODO (mlvdv) report this?
                 } else {
+                    // Get the source line where the
                     final LineLocation lineLocation = srcSection.getLineLocation();
-                    CoverageCounter counter = counters.get(lineLocation);
-                    if (counter != null) {
+                    CoverageRecord record = coverageMap.get(lineLocation);
+                    if (record != null) {
                         // Another node starts on same line; count only the first (textually)
-                        if (srcSection.getCharIndex() > counter.srcSection.getCharIndex()) {
-                            // Counter already in place, corresponds to code earlier on line
+                        if (srcSection.getCharIndex() > record.srcSection.getCharIndex()) {
+                            // Record already in place, corresponds to code earlier on line
                             return;
                         } else {
-                            // Counter already in place, corresponds to later code; replace it
-                            counter.instrument.dispose();
+                            // Record already in place, corresponds to later code; replace it
+                            record.instrument.dispose();
                         }
                     }
                     final AtomicLong count = new AtomicLong();
-                    final CoverageEventReceiver eventReceiver = new CoverageEventReceiver(count);
-                    final Instrument instrument = Instrument.create(eventReceiver, CoverageTracker.class.getSimpleName());
+                    final CoverageEventListener eventListener = new CoverageEventListener(count);
+                    final Instrument instrument = Instrument.create(eventListener, CoverageTracker.class.getSimpleName());
                     instruments.add(instrument);
                     probe.attach(instrument);
-                    counters.put(lineLocation, new CoverageCounter(srcSection, instrument, count));
+                    coverageMap.put(lineLocation, new CoverageRecord(srcSection, instrument, count));
                 }
             }
         }
     }
 
-    private class CoverageCounter {
-        final SourceSection srcSection;
-        final Instrument instrument;
+    private class CoverageRecord {
+
+        final SourceSection srcSection; // The text of the code being counted
+        final Instrument instrument;  // The attached Instrument, in case need to remove.
         final AtomicLong count;
 
-        CoverageCounter(SourceSection srcSection, Instrument instrument, AtomicLong count) {
+        CoverageRecord(SourceSection srcSection, Instrument instrument, AtomicLong count) {
             this.srcSection = srcSection;
             this.instrument = instrument;
             this.count = count;
diff -r 128586040207 -r a2ff253c458f graal/com.oracle.truffle.api/src/com/oracle/truffle/api/tools/NodeExecCounter.java
--- a/graal/com.oracle.truffle.api/src/com/oracle/truffle/api/tools/NodeExecCounter.java	Tue Feb 10 16:44:11 2015 -0800
+++ b/graal/com.oracle.truffle.api/src/com/oracle/truffle/api/tools/NodeExecCounter.java	Tue Feb 10 16:44:19 2015 -0800
@@ -49,7 +49,7 @@
  * <p>
  * <ul>
  * <li>"Execution call" on a node is is defined as invocation of a node method that is instrumented
- * to produce the event {@link TruffleEventReceiver#enter(Node, VirtualFrame)};</li>
+ * to produce the event {@link TruffleEventListener#enter(Node, VirtualFrame)};</li>
  * <li>Execution calls are tabulated only at <em>instrumented</em> nodes, i.e. those for which
  * {@linkplain Node#isInstrumentable() isInstrumentable() == true};</li>
  * <li>Execution calls are tabulated only at nodes present in the AST when originally created;
@@ -92,26 +92,38 @@
     }
 
     /**
-     * Receiver for events at instrumented nodes. Counts are maintained in a shared table, so the
-     * receiver is stateless and can be shared by every {@link Instrument}.
+     * Listener for events at instrumented nodes. Counts are maintained in a shared table, so the
+     * listener is stateless and can be shared by every {@link Instrument}.
      */
-    private final TruffleEventReceiver eventReceiver = new DefaultEventReceiver() {
+    private final TruffleEventListener eventListener = new DefaultEventListener() {
         @Override
         public void enter(Node node, VirtualFrame frame) {
-            internalReceive(node);
+            if (isEnabled()) {
+                final Class<?> nodeClass = node.getClass();
+                /*
+                 * Everything up to here is inlined by Truffle compilation. Delegate the next part
+                 * to a method behind an inlining boundary.
+                 *
+                 * Note that it is not permitted to pass a {@link VirtualFrame} across an inlining
+                 * boundary; they are truly virtual in inlined code.
+                 */
+                AtomicLong nodeCounter = getCounter(nodeClass);
+                nodeCounter.getAndIncrement();
+            }
         }
 
+        /**
+         * Mark this method as a boundary that will stop Truffle inlining, which should not be
+         * allowed to inline the hash table method or any other complex library code.
+         */
         @TruffleBoundary
-        private void internalReceive(Node node) {
-            if (isEnabled()) {
-                final Class<?> nodeClass = node.getClass();
-                AtomicLong nodeCounter = counters.get(nodeClass);
-                if (nodeCounter == null) {
-                    nodeCounter = new AtomicLong();
-                    counters.put(nodeClass, nodeCounter);
-                }
-                nodeCounter.getAndIncrement();
+        private AtomicLong getCounter(Class<?> nodeClass) {
+            AtomicLong nodeCounter = counters.get(nodeClass);
+            if (nodeCounter == null) {
+                nodeCounter = new AtomicLong();
+                counters.put(nodeClass, nodeCounter);
             }
+            return nodeCounter;
         }
     };
 
@@ -268,7 +280,7 @@
 
             if (node.isInstrumentable()) {
                 try {
-                    final Instrument instrument = Instrument.create(eventReceiver, "NodeExecCounter");
+                    final Instrument instrument = Instrument.create(eventListener, "NodeExecCounter");
                     instruments.add(instrument);
                     node.probe().attach(instrument);
                 } catch (ProbeException ex) {
@@ -292,7 +304,7 @@
         @Override
         public void probeTaggedAs(Probe probe, SyntaxTag tag, Object tagValue) {
             if (countingTag == tag) {
-                final Instrument instrument = Instrument.create(eventReceiver, NodeExecCounter.class.getSimpleName());
+                final Instrument instrument = Instrument.create(eventListener, NodeExecCounter.class.getSimpleName());
                 instruments.add(instrument);
                 probe.attach(instrument);
             }