TINKERPOP-2033 Maintain order in profile() annotations Used a synchronized map around a LinkedHashMap rather than ConcurrentHashMap. Not expecting a performance issue with this as its for profile() step which doesn't have to be "fast" per se as it is a debugging step. Affected gryo more than graphson - specifically gryo 1.0 which didn't coerce "metrics" to an interfaces as it does in 3.0. Added a custom serializer to force the annotations to a regular LinkedHashMap and then back to synchronized LinkedHashMap during serder to keep compatibility.
Project: http://git-wip-us.apache.org/repos/asf/tinkerpop/repo Commit: http://git-wip-us.apache.org/repos/asf/tinkerpop/commit/8761118a Tree: http://git-wip-us.apache.org/repos/asf/tinkerpop/tree/8761118a Diff: http://git-wip-us.apache.org/repos/asf/tinkerpop/diff/8761118a Branch: refs/heads/TINKERPOP-2033 Commit: 8761118aac543a5da4f3a990d803fe52fc25ccb5 Parents: e914a67 Author: Stephen Mallette <[email protected]> Authored: Tue Sep 11 12:44:07 2018 -0400 Committer: Stephen Mallette <[email protected]> Committed: Wed Sep 12 12:36:44 2018 -0400 ---------------------------------------------------------------------- CHANGELOG.asciidoc | 1 + .../process/traversal/util/ImmutableMetrics.java | 3 ++- .../structure/io/gryo/GryoSerializersV3d0.java | 8 +++++++- .../gremlin/structure/io/gryo/GryoVersion.java | 3 ++- .../gremlin/structure/io/gryo/UtilSerializers.java | 1 + .../structure/io/gryo/_3_4_0/metrics-v1d0.kryo | Bin 193 -> 197 bytes 6 files changed, 13 insertions(+), 3 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/8761118a/CHANGELOG.asciidoc ---------------------------------------------------------------------- diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index 1d9aa41..16a56e4 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -32,6 +32,7 @@ This release also includes changes from <<release-3-3-3, 3.3.3>>. * Added `GraphFeatures.supportsIoRead()` and `GraphFeatures.supportsIoWrite()`. * Deprecated `Graph.io()` and related infrastructure. * `GraphMLReader` better handles edge and vertex properties with the same name. +* Maintained order of annotations in metrics returned from `profile()`-step. * Bumped to Netty 4.1.25. * Bumped to Spark 2.3.1. * Deprecated two `submit()`-related methods on the Java driver `Client` class. http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/8761118a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/ImmutableMetrics.java ---------------------------------------------------------------------- diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/ImmutableMetrics.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/ImmutableMetrics.java index d2b1430..f633322 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/ImmutableMetrics.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/ImmutableMetrics.java @@ -20,6 +20,7 @@ package org.apache.tinkerpop.gremlin.process.traversal.util; import java.io.Serializable; import java.util.Collection; +import java.util.Collections; import java.util.HashMap; import java.util.LinkedHashMap; import java.util.Map; @@ -41,7 +42,7 @@ public class ImmutableMetrics implements Metrics, Serializable { protected String name; protected Map<String, AtomicLong> counts = new ConcurrentHashMap<>(); protected long durationNs = 0l; - protected final Map<String, Object> annotations = new ConcurrentHashMap<>(); + protected final Map<String, Object> annotations = Collections.synchronizedMap(new LinkedHashMap<>()); protected final Map<String, ImmutableMetrics> nested = new LinkedHashMap<>(); protected ImmutableMetrics() { http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/8761118a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/gryo/GryoSerializersV3d0.java ---------------------------------------------------------------------- diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/gryo/GryoSerializersV3d0.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/gryo/GryoSerializersV3d0.java index 8a57a06..fee9345 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/gryo/GryoSerializersV3d0.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/gryo/GryoSerializersV3d0.java @@ -52,6 +52,7 @@ 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.concurrent.TimeUnit; @@ -427,7 +428,12 @@ public final class GryoSerializersV3d0 { output.writeString(object.getName()); output.writeDouble(object.getDuration(TimeUnit.NANOSECONDS) / 1000000d); kryo.writeObject(output, object.getCounts()); - kryo.writeObject(output, object.getAnnotations()); + + // annotations is a synchronized LinkedHashMap - get rid of the "synch" for serialization as gryo + // doesn't know how to deserialize that well and LinkedHashMap should work with 3.3.x and previous + final Map<String, Object> annotations = new LinkedHashMap<>(); + object.getAnnotations().forEach(annotations::put); + kryo.writeObject(output, annotations); // kryo might have a problem with LinkedHashMap value collections. can't recreate it independently but // it gets fixed with standard collections for some reason. http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/8761118a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/gryo/GryoVersion.java ---------------------------------------------------------------------- diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/gryo/GryoVersion.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/gryo/GryoVersion.java index 39e0cb4..7af3766 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/gryo/GryoVersion.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/gryo/GryoVersion.java @@ -244,6 +244,7 @@ public enum GryoVersion { add(GryoTypeReg.of(Collections.singleton(null).getClass(), 54)); add(GryoTypeReg.of(Collections.singletonList(null).getClass(), 24)); add(GryoTypeReg.of(Collections.singletonMap(null, null).getClass(), 23)); + add(GryoTypeReg.of(Types.COLLECTIONS_SYNCHRONIZED_MAP, 185, new UtilSerializers.SynchronizedMapSerializer())); // ***LAST ID*** add(GryoTypeReg.of(Contains.class, 49)); add(GryoTypeReg.of(Currency.class, 40)); add(GryoTypeReg.of(Date.class, 38)); @@ -406,7 +407,7 @@ public enum GryoVersion { "org.apache.tinkerpop.gremlin.driver.ser.ResponseMessageGryoSerializer", 169); tryAddDynamicType(this, "org.apache.tinkerpop.gremlin.sparql.process.traversal.strategy.SparqlStrategy", - null, 184); // ***LAST ID*** + null, 184); }}; } http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/8761118a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/gryo/UtilSerializers.java ---------------------------------------------------------------------- diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/gryo/UtilSerializers.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/gryo/UtilSerializers.java index 7aec235..eda314c 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/gryo/UtilSerializers.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/gryo/UtilSerializers.java @@ -256,6 +256,7 @@ final class UtilSerializers { map.forEach(m::put); kryo.writeObject(output, m); } + @Override public <I extends InputShim> Map read(final KryoShim<I, ?> kryo, final I input, final Class<Map> clazz) { return Collections.synchronizedMap(kryo.readObject(input, LinkedHashMap.class)); http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/8761118a/gremlin-tools/gremlin-io-test/src/test/resources/org/apache/tinkerpop/gremlin/structure/io/gryo/_3_4_0/metrics-v1d0.kryo ---------------------------------------------------------------------- diff --git a/gremlin-tools/gremlin-io-test/src/test/resources/org/apache/tinkerpop/gremlin/structure/io/gryo/_3_4_0/metrics-v1d0.kryo b/gremlin-tools/gremlin-io-test/src/test/resources/org/apache/tinkerpop/gremlin/structure/io/gryo/_3_4_0/metrics-v1d0.kryo index f660f85..d94cd24 100644 Binary files a/gremlin-tools/gremlin-io-test/src/test/resources/org/apache/tinkerpop/gremlin/structure/io/gryo/_3_4_0/metrics-v1d0.kryo and b/gremlin-tools/gremlin-io-test/src/test/resources/org/apache/tinkerpop/gremlin/structure/io/gryo/_3_4_0/metrics-v1d0.kryo differ
