You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tinkerpop.apache.org by sp...@apache.org on 2017/07/11 12:45:53 UTC

[44/50] [abbrv] tinkerpop git commit: TINKERPOP-1686 WIP toward thread-safety in metrics

TINKERPOP-1686 WIP toward thread-safety in metrics

Cleaned up javadocs a bit. Added synchronization around setMetrics().


Project: http://git-wip-us.apache.org/repos/asf/tinkerpop/repo
Commit: http://git-wip-us.apache.org/repos/asf/tinkerpop/commit/a8ce816b
Tree: http://git-wip-us.apache.org/repos/asf/tinkerpop/tree/a8ce816b
Diff: http://git-wip-us.apache.org/repos/asf/tinkerpop/diff/a8ce816b

Branch: refs/heads/TINKERPOP-1686
Commit: a8ce816b04ae3543d4799e4272cc22d0e2b3d61a
Parents: 4062898
Author: Stephen Mallette <sp...@genoprime.com>
Authored: Wed Jun 7 09:39:07 2017 -0400
Committer: Stephen Mallette <sp...@genoprime.com>
Committed: Tue Jul 11 08:00:57 2017 -0400

----------------------------------------------------------------------
 .../process/traversal/step/Profiling.java       |   6 +-
 .../step/sideEffect/ProfileSideEffectStep.java  |  14 ++-
 .../traversal/step/util/ProfileStep.java        |   3 +
 .../traversal/util/DefaultTraversalMetrics.java | 112 ++++++++++++-------
 .../traversal/util/TraversalMetrics.java        |   8 +-
 5 files changed, 95 insertions(+), 48 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/a8ce816b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/Profiling.java
----------------------------------------------------------------------
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/Profiling.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/Profiling.java
index 3e4ff19..5fa53ee 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/Profiling.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/Profiling.java
@@ -18,11 +18,13 @@
  */
 package org.apache.tinkerpop.gremlin.process.traversal.step;
 
+import org.apache.tinkerpop.gremlin.process.traversal.Step;
+import org.apache.tinkerpop.gremlin.process.traversal.strategy.finalization.ProfileStrategy;
 import org.apache.tinkerpop.gremlin.process.traversal.util.MutableMetrics;
 
 /**
- * A Step can implement this interface in order to receive a reference to the MutableMetrics object for the Step. The
- * MutableMetrics is initialized when the ProfileStrategy executes.
+ * A {@link Step} can implement this interface in order to receive a reference to the {@link MutableMetrics} object
+ * for the {@link Step}. The {@link MutableMetrics} is initialized when the {@link ProfileStrategy} executes.
  *
  * @author Bob Briody (http://bobbriody.com)
  */

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/a8ce816b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/ProfileSideEffectStep.java
----------------------------------------------------------------------
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/ProfileSideEffectStep.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/ProfileSideEffectStep.java
index be60808..5a60d02 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/ProfileSideEffectStep.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/ProfileSideEffectStep.java
@@ -45,7 +45,7 @@ public final class ProfileSideEffectStep<S> extends SideEffectStep<S> implements
     }
 
     @Override
-    protected void sideEffect(Traverser.Admin<S> traverser) {
+    protected void sideEffect(final Traverser.Admin<S> traverser) {
     }
 
     @Override
@@ -61,7 +61,8 @@ public final class ProfileSideEffectStep<S> extends SideEffectStep<S> implements
             return start;
         } finally {
             if (!this.onGraphComputer && start == null) {
-                ((DefaultTraversalMetrics) this.getTraversal().getSideEffects().get(this.sideEffectKey)).setMetrics(this.getTraversal(), false);
+                final DefaultTraversalMetrics m = getTraversalMetricsFromSideEffects();
+                if (!m.isFinalized()) m.setMetrics(this.getTraversal(), false);
             }
         }
     }
@@ -70,14 +71,19 @@ public final class ProfileSideEffectStep<S> extends SideEffectStep<S> implements
     public boolean hasNext() {
         boolean start = super.hasNext();
         if (!this.onGraphComputer && !start) {
-            ((DefaultTraversalMetrics) this.getTraversal().getSideEffects().get(this.sideEffectKey)).setMetrics(this.getTraversal(), false);
+            final DefaultTraversalMetrics m = getTraversalMetricsFromSideEffects();
+            if (!m.isFinalized()) m.setMetrics(this.getTraversal(), false);
         }
         return start;
     }
 
+    private DefaultTraversalMetrics getTraversalMetricsFromSideEffects() {
+        return (DefaultTraversalMetrics) this.getTraversal().getSideEffects().get(this.sideEffectKey);
+    }
+
     @Override
     public DefaultTraversalMetrics generateFinalResult(final DefaultTraversalMetrics tm) {
-        if (this.onGraphComputer)
+        if (this.onGraphComputer && !tm.isFinalized())
             tm.setMetrics(this.getTraversal(), true);
         return tm;
     }

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/a8ce816b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/ProfileStep.java
----------------------------------------------------------------------
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/ProfileStep.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/ProfileStep.java
index 9b2276b..0bc5f4f 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/ProfileStep.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/ProfileStep.java
@@ -90,6 +90,9 @@ public final class ProfileStep<S> extends AbstractStep<S, S> implements MemoryCo
             this.onGraphComputer = TraversalHelper.onGraphComputer(this.getTraversal());
             this.metrics = new MutableMetrics(this.getPreviousStep().getId(), this.getPreviousStep().toString());
             final Step<?, S> previousStep = this.getPreviousStep();
+
+            // give metrics to the step being profiled so that it can add additional data to the metrics like
+            // annotations
             if (previousStep instanceof Profiling)
                 ((Profiling) previousStep).setMetrics(this.metrics);
         }

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/a8ce816b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/DefaultTraversalMetrics.java
----------------------------------------------------------------------
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/DefaultTraversalMetrics.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/DefaultTraversalMetrics.java
index 3c543f6..df24932 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/DefaultTraversalMetrics.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/DefaultTraversalMetrics.java
@@ -28,7 +28,6 @@ import java.io.Serializable;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.HashMap;
-import java.util.Iterator;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
@@ -37,6 +36,9 @@ import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicInteger;
 
 /**
+ * Default implementation for {@link TraversalMetrics} that aggregates {@link ImmutableMetrics} instances from a
+ * {@link Traversal}.
+ *
  * @author Bob Briody (http://bobbriody.com)
  * @author Marko A. Rodriguez (http://markorodriguez.com)
  */
@@ -49,12 +51,21 @@ public final class DefaultTraversalMetrics implements TraversalMetrics, Serializ
     private final Map<String, MutableMetrics> metrics = new HashMap<>();
     private final TreeMap<Integer, String> indexToLabelMap = new TreeMap<>();
 
-    /*
-    The following are computed values upon the completion of profiling in order to report the results back to the user
+    /**
+     * A computed value representing the total time spent on all steps.
      */
     private long totalStepDuration;
+
+    /**
+     * The metrics that are reported to the caller of profile() which are computed once all metrics have been gathered.
+     */
     private Map<String, ImmutableMetrics> computedMetrics = new LinkedHashMap<>();
 
+    /**
+     * Determines if final metrics have been computed
+     */
+    private volatile boolean finalized = false;
+
     public DefaultTraversalMetrics() {
     }
 
@@ -92,6 +103,10 @@ public final class DefaultTraversalMetrics implements TraversalMetrics, Serializ
         return this.computedMetrics.values();
     }
 
+    public boolean isFinalized() {
+        return finalized;
+    }
+
     @Override
     public String toString() {
         // Build a pretty table of metrics data.
@@ -111,6 +126,18 @@ public final class DefaultTraversalMetrics implements TraversalMetrics, Serializ
         return sb.toString();
     }
 
+    /**
+     * Extracts metrics from the provided {@code traversal} and computes metrics. Calling this method finalizes the
+     * metrics such that their values can no longer be modified.
+     */
+    public synchronized void setMetrics(final Traversal.Admin traversal, final boolean onGraphComputer) {
+        if (finalized) throw new IllegalStateException("Metrics have been finalized");
+        finalized = true;
+        addTopLevelMetrics(traversal, onGraphComputer);
+        handleNestedTraversals(traversal, null, onGraphComputer);
+        computeTotals();
+    }
+
     private void appendMetrics(final Collection<? extends Metrics> metrics, final StringBuilder sb, final int indent) {
         // Append each StepMetric's row. indexToLabelMap values are ordered by index.
         for (Metrics m : metrics) {
@@ -222,41 +249,7 @@ public final class DefaultTraversalMetrics implements TraversalMetrics, Serializ
         tempMetrics.forEach(it -> this.computedMetrics.put(it.getId(), it.getImmutableClone()));
     }
 
-    public static DefaultTraversalMetrics merge(final Iterator<DefaultTraversalMetrics> toMerge) {
-        final DefaultTraversalMetrics newTraversalMetrics = new DefaultTraversalMetrics();
-
-        // iterate the incoming TraversalMetrics
-        toMerge.forEachRemaining(inTraversalMetrics -> {
-            // aggregate the internal Metrics
-            inTraversalMetrics.metrics.forEach((metricsId, toAggregate) -> {
-
-                MutableMetrics aggregateMetrics = newTraversalMetrics.metrics.get(metricsId);
-                if (null == aggregateMetrics) {
-                    // need to create a Metrics to aggregate into
-                    aggregateMetrics = new MutableMetrics(toAggregate.getId(), toAggregate.getName());
-
-                    newTraversalMetrics.metrics.put(metricsId, aggregateMetrics);
-                    // Set the index of the Metrics
-                    for (final Map.Entry<Integer, String> entry : inTraversalMetrics.indexToLabelMap.entrySet()) {
-                        if (metricsId.equals(entry.getValue())) {
-                            newTraversalMetrics.indexToLabelMap.put(entry.getKey(), metricsId);
-                            break;
-                        }
-                    }
-                }
-                aggregateMetrics.aggregate(toAggregate);
-            });
-        });
-        return newTraversalMetrics;
-    }
-
-    public void setMetrics(final Traversal.Admin traversal, final boolean onGraphComputer) {
-        addTopLevelMetrics(traversal, onGraphComputer);
-        handleNestedTraversals(traversal, null, onGraphComputer);
-        computeTotals();
-    }
-
-    private void addTopLevelMetrics(Traversal.Admin traversal, final boolean onGraphComputer) {
+    private void addTopLevelMetrics(final Traversal.Admin traversal, final boolean onGraphComputer) {
         final List<ProfileStep> profileSteps = TraversalHelper.getStepsOfClass(ProfileStep.class, traversal);
         for (int ii = 0; ii < profileSteps.size(); ii++) {
             // The index is necessary to ensure that step order is preserved after a merge.
@@ -308,4 +301,47 @@ public final class DefaultTraversalMetrics implements TraversalMetrics, Serializ
             }
         }
     }
+
+    private void appendMetrics(final Collection<? extends Metrics> metrics, final StringBuilder sb, final int indent) {
+        // Append each StepMetric's row. indexToLabelMap values are ordered by index.
+        for (Metrics m : metrics) {
+            String rowName = m.getName();
+
+            // Handle indentation
+            for (int ii = 0; ii < indent; ii++) {
+                rowName = "  " + rowName;
+            }
+            // Abbreviate if necessary
+            rowName = StringUtils.abbreviate(rowName, 50);
+
+            // Grab the values
+            final Long itemCount = m.getCount(ELEMENT_COUNT_ID);
+            final Long traverserCount = m.getCount(TRAVERSER_COUNT_ID);
+            Double percentDur = (Double) m.getAnnotation(PERCENT_DURATION_KEY);
+
+            // Build the row string
+
+            sb.append(String.format("%n%-50s", rowName));
+
+            if (itemCount != null) {
+                sb.append(String.format(" %21d", itemCount));
+            } else {
+                sb.append(String.format(" %21s", ""));
+            }
+
+            if (traverserCount != null) {
+                sb.append(String.format(" %11d", traverserCount));
+            } else {
+                sb.append(String.format(" %11s", ""));
+            }
+
+            sb.append(String.format(" %15.3f", m.getDuration(TimeUnit.MICROSECONDS) / 1000.0));
+
+            if (percentDur != null) {
+                sb.append(String.format(" %8.2f", percentDur));
+            }
+
+            appendMetrics(m.getNested(), sb, indent + 1);
+        }
+    }
 }

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/a8ce816b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/TraversalMetrics.java
----------------------------------------------------------------------
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/TraversalMetrics.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/TraversalMetrics.java
index 6a54680..0dbb2f6 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/TraversalMetrics.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/TraversalMetrics.java
@@ -46,15 +46,13 @@ public interface TraversalMetrics {
     /**
      * Get the total duration taken by the Traversal.
      *
-     * @param unit
      * @return total duration taken by the Traversal.
      */
-    public long getDuration(TimeUnit unit);
+    public long getDuration(final TimeUnit unit);
 
     /**
      * Get an individual Metrics object by the index of the profiled Step.
      *
-     * @param stepIndex
      * @return an individual Metrics object.
      */
     public Metrics getMetrics(final int stepIndex);
@@ -62,10 +60,12 @@ public interface TraversalMetrics {
     /**
      * Get an individual Metrics object by the id of the profiled Step.
      *
-     * @param id
      * @return an individual Metrics object.
      */
     public Metrics getMetrics(final String id);
 
+    /**
+     * Gets all the metrics.
+     */
     public Collection<? extends Metrics> getMetrics();
 }