You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tinkerpop.apache.org by spmallette <gi...@git.apache.org> on 2017/07/11 17:09:57 UTC
[GitHub] tinkerpop pull request #669: TINKERPOP-1686 Thread safe Traversal Metrics
GitHub user spmallette opened a pull request:
https://github.com/apache/tinkerpop/pull/669
TINKERPOP-1686 Thread safe Traversal Metrics
https://issues.apache.org/jira/browse/TINKERPOP-1686
Improved the thread safety of traversal metrics. Some graph providers may write to these metrics from multiple threads and it may thus cause problems. These changes had to occur on 3.3.0 when a break was allowable as the internals of the class itself had to change and that bound the related classes to Gryo 1.0 compatibility along the 3.2.x line.
(I just wanted to get the PR issued - tests are still running. I will update this when ready for review)
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/apache/tinkerpop TINKERPOP-1686
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/tinkerpop/pull/669.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #669
----
commit a8ce816b04ae3543d4799e4272cc22d0e2b3d61a
Author: Stephen Mallette <sp...@genoprime.com>
Date: 2017-06-07T13:39:07Z
TINKERPOP-1686 WIP toward thread-safety in metrics
Cleaned up javadocs a bit. Added synchronization around setMetrics().
commit f5ffa0d8237668613dade85c19b51bf285e6d72e
Author: Stephen Mallette <sp...@genoprime.com>
Date: 2017-06-07T17:42:56Z
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.
commit 4a5efabc07700f5c497f0e8e2e57c13dc8826359
Author: Stephen Mallette <sp...@genoprime.com>
Date: 2017-06-07T19:06:10Z
TINKERPOP-1686 Refactored DefaultTraversalMetrics
There seems to be some weird problem in gryo serialization for this class that is difficult to reproduce. It seemed to have something to do with nested maps or perhaps TreeMap, but I wasn't able to easily isolate it to create a bug report to Kryo. All I know is that these changes made the serialization failure (a buffer underflow) go away. It also fixed a problem where deserialization with GraphSON did not allow the getMetrics(int) method to work.
commit 4ec69216c9ab23bf09eb9434d96ab6fb3ca14726
Author: Stephen Mallette <sp...@genoprime.com>
Date: 2017-06-19T20:33:03Z
TINKERPOP-1686 No need for LinkedHashMap
Order can be determined by the key which is the step number essentially. Had to regenerate the model for IO tests, but this ends up being a breaking change in gryo.
commit c5364356bd40e14dfba93f5b32b04628aa5b9234
Author: Stephen Mallette <sp...@genoprime.com>
Date: 2017-06-28T11:38:40Z
TINKERPOP-1686 Update changelog
commit d136455b83955a81440d97b5fa26bd5d60f23f4b
Author: Stephen Mallette <sp...@genoprime.com>
Date: 2017-06-28T11:55:51Z
TINKERPOP-1686 Made MutableMetrics thread-safe for nested results and counts
commit 19a573c3c5dcf53bd51b539800a3bc923fda73c9
Author: Stephen Mallette <sp...@genoprime.com>
Date: 2017-06-28T12:27:19Z
TINKERPOP-1686 Updated Gryo serialization tests for metrics
commit dee27e33725337c9cddad33f6c3f8cd310203a3c
Author: Stephen Mallette <sp...@genoprime.com>
Date: 2017-07-11T12:44:26Z
TINKERPOP-1686 Regenerated gryo files for metrics after gryo 3.0 work
commit 773267f19a37f86e8a19ef68a390a846d76b5d02
Author: Stephen Mallette <sp...@genoprime.com>
Date: 2017-07-11T13:50:43Z
TINKERPOP-1686 Fixed up gryo io docs for 3.0
commit 580af332833e5dcf6b0e29459ff9a706b69947f8
Author: Stephen Mallette <sp...@genoprime.com>
Date: 2017-07-11T13:51:14Z
TINKERPOP-1686 Cleaned up registrations a bit for Gryo 3.0
commit 85dc8a98444017f7a45b13b4460595725684badc
Author: Stephen Mallette <sp...@genoprime.com>
Date: 2017-07-11T16:24:45Z
TINKERPOP-1686 Add some javadoc and synchronize some methods for thread safety
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tinkerpop pull request #669: TINKERPOP-1686 Thread safe Traversal Metrics
Posted by dkuppitz <gi...@git.apache.org>.
Github user dkuppitz commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/669#discussion_r127075299
--- Diff: gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/DefaultTraversalMetrics.java ---
@@ -23,52 +23,64 @@
import org.apache.tinkerpop.gremlin.process.traversal.Traversal;
import org.apache.tinkerpop.gremlin.process.traversal.step.TraversalParent;
import org.apache.tinkerpop.gremlin.process.traversal.step.util.ProfileStep;
+import org.javatuples.Pair;
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;
-import java.util.TreeMap;
import java.util.concurrent.TimeUnit;
-import java.util.concurrent.atomic.AtomicInteger;
+import java.util.stream.Collectors;
/**
+ * 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)
+ * @author Stephen Mallette (http://stephen.genoprime.com)
*/
public final class DefaultTraversalMetrics implements TraversalMetrics, Serializable {
/**
* toString() specific headers
*/
private static final String[] HEADERS = {"Step", "Count", "Traversers", "Time (ms)", "% Dur"};
- private final Map<String, MutableMetrics> metrics = new HashMap<>();
- private final TreeMap<Integer, String> indexToLabelMap = new TreeMap<>();
+ /**
+ * {@link ImmutableMetrics} indexed by their step identifier.
+ */
+ private final Map<String, ImmutableMetrics> stepIndexedMetrics = new HashMap<>();
- /*
- 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;
- private Map<String, ImmutableMetrics> computedMetrics = new LinkedHashMap<>();
+
+ /**
+ * {@link ImmutableMetrics} indexed by their step position.
+ */
+ private Map<Integer, ImmutableMetrics> positionIndexedMetrics = new HashMap<>();
+
+ /**
+ * Determines if final metrics have been computed
+ */
+ private volatile boolean finalized = false;
public DefaultTraversalMetrics() {
}
/**
* This is only a convenient constructor needed for GraphSON deserialization.
*/
- public DefaultTraversalMetrics(final long totalStepDurationNs, final List<MutableMetrics> metricsMap) {
- this.totalStepDuration = totalStepDurationNs;
- this.computedMetrics = new LinkedHashMap<>(this.metrics.size());
- final AtomicInteger counter = new AtomicInteger(0);
- metricsMap.forEach(metric -> {
- this.computedMetrics.put(metric.getId(), metric.getImmutableClone());
- this.indexToLabelMap.put(counter.getAndIncrement(), metric.getId());
- });
+ public DefaultTraversalMetrics(final long totalStepDurationNs, final List<MutableMetrics> orderedMetrics) {
+ totalStepDuration = totalStepDurationNs;
+ for (int ix = 0; ix < orderedMetrics.size(); ix++) {
--- End diff --
Good that `orderedMetrics.forEach()` was replaced with a simple `for` loop, but I'm not sure if calling get(ix) 3 times really makes it more efficient. Maybe:
```
int ix = 0;
for (final MutableMetrics metric : orderedMetrics) {
stepIndexedMetrics.put(metric.getId(), metric.getImmutableClone());
positionIndexedMetrics.put(ix++, metric.getImmutableClone());
}
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tinkerpop pull request #669: TINKERPOP-1686 Thread safe Traversal Metrics
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/tinkerpop/pull/669
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tinkerpop issue #669: TINKERPOP-1686 Thread safe Traversal Metrics
Posted by twilmes <gi...@git.apache.org>.
Github user twilmes commented on the issue:
https://github.com/apache/tinkerpop/pull/669
VOTE: +1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---