From 208910695b3b37bfc7a80c3cdfe945c931edf2c5 Mon Sep 17 00:00:00 2001 From: rjbriody Date: Thu, 7 Jan 2016 15:52:19 -0500 Subject: [PATCH] Fix the way DependantMutableMetrics profiler durations are calculated. --- .../util/DependantMutableMetrics.java | 34 +++++++++++-------- .../traversal/util/MutableMetrics.java | 3 +- 2 files changed, 21 insertions(+), 16 deletions(-) diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/DependantMutableMetrics.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/DependantMutableMetrics.java index fd580fe0267..0ce939a6354 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/DependantMutableMetrics.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/DependantMutableMetrics.java @@ -18,9 +18,11 @@ */ package org.apache.tinkerpop.gremlin.process.traversal.util; +import java.util.concurrent.TimeUnit; + /** * This Metrics class handles a metrics chain in which durations are "double counted" by upstream metrics. Durations are - * corrected on-the-fly by subtracting upstream durations on every call to stop(). + * corrected upon retrieval by subtracting upstream durations. * * @author Bob Briody (http://bobbriody.com) */ @@ -38,22 +40,24 @@ public DependantMutableMetrics(final String id, final String name, final Dependa this.upStreamMetrics = upStreamMetrics; } - public void start() { - super.start(); - } - - public void stop() { - super.stop(); - // root step will not have an upstream metrics - if (upStreamMetrics != null) { - // subtract time that is "double counted" by upstream metrics - super.durationNs -= upStreamMetrics.getAndResetIncrementalDur(); + /** + * Returns the actual duration taken by this Metrics by subtracting the duration taken by the upstream Step, if one exists. + * @param unit + * @return + */ + @Override + public long getDuration(final TimeUnit unit) { + if (upStreamMetrics == null){ + return unit.convert(super.durationNs, unit); + } else { + // upStreamMetrics exists. Subtract that duration since it is time not spent in this step. + return unit.convert(super.durationNs - upStreamMetrics.durationNs, unit); } } - public long getAndResetIncrementalDur() { - long incrementalDur = super.durationNs - prevDur; - prevDur = super.durationNs; - return incrementalDur; + @Override + protected void copyMembers(final ImmutableMetrics clone) { + super.copyMembers(clone); + clone.durationNs = this.getDuration(TimeUnit.NANOSECONDS); } } 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 53050201c61..72c1076843e 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 @@ -157,9 +157,10 @@ public ImmutableMetrics getImmutableClone() { return clone; } - private void copyMembers(final ImmutableMetrics clone) { + protected void copyMembers(final ImmutableMetrics clone) { clone.id = this.id; clone.name = this.name; + // Note: This value is overwritten in the DependantMutableMetrics overridden copyMembers method. clone.durationNs = this.durationNs; for (Map.Entry c : this.counts.entrySet()) { clone.counts.put(c.getKey(), new AtomicLong(c.getValue().get()));