Repository: tinkerpop Updated Branches: refs/heads/TINKERPOP-2033 [created] 410e6d5a7
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/410e6d5a Tree: http://git-wip-us.apache.org/repos/asf/tinkerpop/tree/410e6d5a Diff: http://git-wip-us.apache.org/repos/asf/tinkerpop/diff/410e6d5a Branch: refs/heads/TINKERPOP-2033 Commit: 410e6d5a7877a9568ec190624ecd12c81e11418d Parents: a9bc444 Author: Stephen Mallette <[email protected]> Authored: Tue Sep 11 12:44:07 2018 -0400 Committer: Stephen Mallette <[email protected]> Committed: Wed Sep 12 08:35:11 2018 -0400 ---------------------------------------------------------------------- CHANGELOG.asciidoc | 2 ++ .../traversal/util/ImmutableMetrics.java | 3 ++- .../structure/io/gryo/GryoSerializersV3d0.java | 8 +++++++- .../gremlin/structure/io/gryo/GryoVersion.java | 15 +++++++++++++-- .../structure/io/gryo/UtilSerializers.java | 19 +++++++++++++++++++ .../structure/io/gryo/_3_4_0/metrics-v1d0.kryo | Bin 193 -> 197 bytes 6 files changed, 43 insertions(+), 4 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/410e6d5a/CHANGELOG.asciidoc ---------------------------------------------------------------------- diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index 4e00d80..117e6bf 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -32,6 +32,8 @@ 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. +* Added synchronized `Map` to Gryo 1.0 and 3.0 registrations. * 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/410e6d5a/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/410e6d5a/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/410e6d5a/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 6d2d676..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); }}; } @@ -435,6 +436,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)); @@ -524,7 +526,7 @@ public enum GryoVersion { add(GryoTypeReg.of(MapReduce.NullObject.class, 74)); add(GryoTypeReg.of(AtomicLong.class, 79)); add(GryoTypeReg.of(Pair.class, 88, new UtilSerializers.PairSerializer())); - add(GryoTypeReg.of(Triplet.class, 183, new UtilSerializers.TripletSerializer())); // ***LAST ID*** + add(GryoTypeReg.of(Triplet.class, 183, new UtilSerializers.TripletSerializer())); add(GryoTypeReg.of(TraversalExplanation.class, 106, new JavaSerializer())); add(GryoTypeReg.of(Duration.class, 93, new JavaTimeSerializers.DurationSerializer())); @@ -629,6 +631,8 @@ public enum GryoVersion { private static final Class HASH_MAP_TREE_NODE; + private static final Class COLLECTIONS_SYNCHRONIZED_MAP; + static { // have to instantiate this via reflection because it is a private inner class of HashMap String className = HashMap.class.getName() + "$Node"; @@ -644,6 +648,13 @@ public enum GryoVersion { } catch (Exception ex) { throw new RuntimeException("Could not access " + className, ex); } + + className = Collections.class.getName() + "$SynchronizedMap"; + try { + COLLECTIONS_SYNCHRONIZED_MAP = Class.forName(className); + } catch (Exception ex) { + throw new RuntimeException("Could not access " + className, ex); + } } private Types() {} http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/410e6d5a/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 aff6059..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 @@ -36,6 +36,8 @@ import java.nio.ByteBuffer; import java.util.AbstractMap; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.UUID; @@ -243,4 +245,21 @@ final class UtilSerializers { return new AbstractMap.SimpleEntry(kryo.readClassAndObject(input), kryo.readClassAndObject(input)); } } + + /** + * Serializer for {@code List} instances produced by {@code Arrays.asList()}. + */ + final static class SynchronizedMapSerializer implements SerializerShim<Map> { + @Override + public <O extends OutputShim> void write(final KryoShim<?, O> kryo, final O output, final Map map) { + final Map m = new LinkedHashMap(); + 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/410e6d5a/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
