TINKERPOP-1686 Minor tweaks to finalize MutableMetrics MutableMetrics attached to DefaultTraversalMetrics should not keep getting stuff written to them after the DefaultTraversalMetrics have been finalized.
Project: http://git-wip-us.apache.org/repos/asf/tinkerpop/repo Commit: http://git-wip-us.apache.org/repos/asf/tinkerpop/commit/40884405 Tree: http://git-wip-us.apache.org/repos/asf/tinkerpop/tree/40884405 Diff: http://git-wip-us.apache.org/repos/asf/tinkerpop/diff/40884405 Branch: refs/heads/TINKERPOP-1686 Commit: 4088440507fa4f3682b0058f76419a7bb03e7cc8 Parents: 909c1f6 Author: Stephen Mallette <[email protected]> Authored: Wed Jun 7 13:42:56 2017 -0400 Committer: Stephen Mallette <[email protected]> Committed: Wed Jun 28 08:31:46 2017 -0400 ---------------------------------------------------------------------- .../traversal/util/DefaultTraversalMetrics.java | 7 +++- .../process/traversal/util/MutableMetrics.java | 41 ++++++++++++++++---- 2 files changed, 38 insertions(+), 10 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/40884405/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 0f19ecc..ed2aa2d 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 @@ -98,6 +98,9 @@ public final class DefaultTraversalMetrics implements TraversalMetrics, Serializ return this.computedMetrics.values(); } + /** + * The metrics have been computed and can no longer be modified. + */ public boolean isFinalized() { return finalized; } @@ -126,7 +129,7 @@ public final class DefaultTraversalMetrics implements TraversalMetrics, Serializ * 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"); + if (finalized) throw new IllegalStateException("Metrics have been finalized and cannot be modified"); finalized = true; addTopLevelMetrics(traversal, onGraphComputer); handleNestedTraversals(traversal, null, onGraphComputer); @@ -275,7 +278,7 @@ public final class DefaultTraversalMetrics implements TraversalMetrics, Serializ if (null != metrics) { // this happens when a particular branch never received a .next() call (the metrics were never initialized) if (!onGraphComputer) { // subtract upstream duration. - long durBeforeAdjustment = metrics.getDuration(TimeUnit.NANOSECONDS); + final long durBeforeAdjustment = metrics.getDuration(TimeUnit.NANOSECONDS); // adjust duration metrics.setDuration(metrics.getDuration(TimeUnit.NANOSECONDS) - prevDur, TimeUnit.NANOSECONDS); prevDur = durBeforeAdjustment; http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/40884405/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/MutableMetrics.java ---------------------------------------------------------------------- diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/MutableMetrics.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/MutableMetrics.java index 47decf1..34fa370 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/MutableMetrics.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/MutableMetrics.java @@ -18,6 +18,8 @@ */ package org.apache.tinkerpop.gremlin.process.traversal.util; +import org.apache.tinkerpop.gremlin.process.traversal.step.Profiling; + import java.util.Arrays; import java.util.List; import java.util.Map; @@ -33,6 +35,11 @@ public class MutableMetrics extends ImmutableMetrics implements Cloneable { private long tempTime = -1L; + /** + * Determines if metrics have been finalized, meaning that no more may be collected. + */ + private volatile boolean finalized = false; + protected MutableMetrics() { // necessary for gryo serialization } @@ -55,10 +62,12 @@ public class MutableMetrics extends ImmutableMetrics implements Cloneable { } public void addNested(final MutableMetrics metrics) { + if (finalized) throw new IllegalStateException("Metrics have been finalized and cannot be modified"); this.nested.put(metrics.getId(), metrics); } public void start() { + if (finalized) throw new IllegalStateException("Metrics have been finalized and cannot be modified"); if (-1 != this.tempTime) { throw new IllegalStateException("Internal Error: Concurrent Metrics start. Stop timer before starting timer."); } @@ -66,6 +75,7 @@ public class MutableMetrics extends ImmutableMetrics implements Cloneable { } public void stop() { + if (finalized) throw new IllegalStateException("Metrics have been finalized and cannot be modified"); if (-1 == this.tempTime) throw new IllegalStateException("Internal Error: Metrics has not been started. Start timer before stopping timer"); this.durationNs = this.durationNs + (System.nanoTime() - this.tempTime); @@ -73,6 +83,7 @@ public class MutableMetrics extends ImmutableMetrics implements Cloneable { } public void incrementCount(final String key, final long incr) { + if (finalized) throw new IllegalStateException("Metrics have been finalized and cannot be modified"); AtomicLong count = this.counts.get(key); if (count == null) { count = new AtomicLong(); @@ -82,14 +93,17 @@ public class MutableMetrics extends ImmutableMetrics implements Cloneable { } public void setDuration(final long dur, final TimeUnit unit) { + if (finalized) throw new IllegalStateException("Metrics have been finalized and cannot be modified"); this.durationNs = TimeUnit.NANOSECONDS.convert(dur, unit); } public void setCount(final String key, final long val) { + if (finalized) throw new IllegalStateException("Metrics have been finalized and cannot be modified"); this.counts.put(key, new AtomicLong(val)); } public void aggregate(final MutableMetrics other) { + if (finalized) throw new IllegalStateException("Metrics have been finalized and cannot be modified"); this.durationNs += other.durationNs; for (Map.Entry<String, AtomicLong> otherCount : other.counts.entrySet()) { AtomicLong thisCount = this.counts.get(otherCount.getKey()); @@ -114,12 +128,11 @@ public class MutableMetrics extends ImmutableMetrics implements Cloneable { } } else { // Numbers are summed - Number existingNum = (Number) existingVal; - Number otherNum = (Number) p.getValue(); + final Number existingNum = (Number) existingVal; + final Number otherNum = (Number) p.getValue(); Number newVal; if (existingNum instanceof Double || existingNum instanceof Float) { - newVal = - existingNum.doubleValue() + otherNum.doubleValue(); + newVal = existingNum.doubleValue() + otherNum.doubleValue(); } else { newVal = existingNum.longValue() + otherNum.longValue(); } @@ -145,11 +158,9 @@ public class MutableMetrics extends ImmutableMetrics implements Cloneable { /** * Set an annotation value. Support exists for Strings and Numbers only. During a merge, Strings are concatenated * into a "," (comma) separated list of distinct values (duplicates are ignored), and Numbers are summed. - * - * @param key - * @param value */ public void setAnnotation(final String key, final Object value) { + if (finalized) throw new IllegalStateException("Metrics have been finalized and cannot be modified"); if (!(value instanceof String) && !(value instanceof Number)) { throw new IllegalArgumentException("Metrics annotations only support String and Number values."); } @@ -161,7 +172,21 @@ public class MutableMetrics extends ImmutableMetrics implements Cloneable { return (MutableMetrics) nested.get(metricsId); } - public ImmutableMetrics getImmutableClone() { + /** + * Once these metrics are used in computing the final metrics to report through {@link TraversalMetrics} they + * should no longer be modified and are thus finalized. + */ + public boolean isFinalized() { + return finalized; + } + + /** + * Gets a copy of the metrics that is immutable. Once this clone is made, the {@link MutableMetrics} can no + * longer be modified themselves. This prevents custom steps that implement {@link Profiling} from adding to + * the metrics after the traversal is complete. + */ + public synchronized ImmutableMetrics getImmutableClone() { + finalized = true; final ImmutableMetrics clone = new ImmutableMetrics(); copyMembers(clone); this.nested.values().forEach(nested -> clone.nested.put(nested.id, ((MutableMetrics) nested).getImmutableClone()));
