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/909c1f6d Tree: http://git-wip-us.apache.org/repos/asf/tinkerpop/tree/909c1f6d Diff: http://git-wip-us.apache.org/repos/asf/tinkerpop/diff/909c1f6d Branch: refs/heads/TINKERPOP-1686 Commit: 909c1f6d6462609d537db89adde92c291d7ff449 Parents: ea20ca1 Author: Stephen Mallette <[email protected]> Authored: Wed Jun 7 09:39:07 2017 -0400 Committer: Stephen Mallette <[email protected]> Committed: Wed Jun 28 08:31:46 2017 -0400 ---------------------------------------------------------------------- .../process/traversal/step/Profiling.java | 6 +- .../step/sideEffect/ProfileSideEffectStep.java | 14 ++- .../traversal/step/util/ProfileStep.java | 3 + .../traversal/util/DefaultTraversalMetrics.java | 114 ++++++++++++------- .../traversal/util/TraversalMetrics.java | 8 +- 5 files changed, 96 insertions(+), 49 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/909c1f6d/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/909c1f6d/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/909c1f6d/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/909c1f6d/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 c5f290a..0f19ecc 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; @@ -36,6 +35,9 @@ import java.util.TreeMap; import java.util.concurrent.TimeUnit; /** + * 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) */ @@ -48,12 +50,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; + /** + * Determines if final metrics have been computed + */ + private volatile boolean finalized = false; + public DefaultTraversalMetrics() { } @@ -62,7 +73,7 @@ public final class DefaultTraversalMetrics implements TraversalMetrics, Serializ */ public DefaultTraversalMetrics(final long totalStepDurationNs, final List<MutableMetrics> metricsMap) { this.totalStepDuration = totalStepDurationNs; - this.computedMetrics = new LinkedHashMap<>(this.metrics.size()); + this.computedMetrics = new LinkedHashMap<>(metricsMap.size()); metricsMap.forEach(metric -> this.computedMetrics.put(metric.getId(), metric.getImmutableClone())); } @@ -87,6 +98,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. @@ -106,6 +121,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) { @@ -217,41 +244,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. @@ -303,4 +296,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/909c1f6d/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(); }
