[GitHub] jorgebay opened a new pull request #1065: TINKERPOP-2161 - GraphBinary: use a single buffer instead of allocators
jorgebay opened a new pull request #1065: TINKERPOP-2161 - GraphBinary: use a single buffer instead of allocators URL: https://github.com/apache/tinkerpop/pull/1065 Improve write path by reusing the same buffer. For serializers this is an API change but given that GraphBinary was introduced recently, I think we can get away with it. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[tinkerpop] branch TINKERPOP-2161 created (now 2402939)
This is an automated email from the ASF dual-hosted git repository. jorgebg pushed a change to branch TINKERPOP-2161 in repository https://gitbox.apache.org/repos/asf/tinkerpop.git. at 2402939 TINKERPOP-2161 GraphBinary: use a single buffer instead of allocators This branch includes the following new commits: new 2402939 TINKERPOP-2161 GraphBinary: use a single buffer instead of allocators The 1 revisions listed above as "new" are entirely new to this repository and will be described in separate emails. The revisions listed as "add" were already present in the repository and have only been added to this reference.
[GitHub] nastra commented on issue #1062: TINKERPOP-2163 Improved performance of JavaTranslator method selection
nastra commented on issue #1062: TINKERPOP-2163 Improved performance of
JavaTranslator method selection
URL: https://github.com/apache/tinkerpop/pull/1062#issuecomment-464780672
@spmallette one tiny suggestion to avoid creation of unnecessary arrays
would be:
```
if (arguments.length > 0) {
final Object[] argumentsCopy = new Object[arguments.length];
for (int i = 0; i < arguments.length; i++) {
argumentsCopy[i] = translateObject(arguments[i]);
}
}
```
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] dkuppitz opened a new pull request #1064: TINKERPOP-2159 EventStrategy doesn't handle multi-valued properties
dkuppitz opened a new pull request #1064: TINKERPOP-2159 EventStrategy doesn't handle multi-valued properties URL: https://github.com/apache/tinkerpop/pull/1064 https://issues.apache.org/jira/browse/TINKERPOP-2159 Fixed multi-valued property handling in `EventStrategy` / `AddPropertyStep`. The `EventStrategy` output for the traversals added in `TinkerGraphPlayTest.testPlayDK()` is as follows: ``` Vertex [v[1]] added to graph [tinkergraph[vertices:1 edges:0]] Vertex [v[1]] property [vp[name->null]] change to [name1] in graph [tinkergraph[vertices:1 edges:0]] Vertex [v[1]] property [vp[name->name1]] change to [name2] in graph [tinkergraph[vertices:1 edges:0]] Vertex [v[1]] property [vp[name->name2]] change to [name2] in graph [tinkergraph[vertices:1 edges:0]] Vertex [v[2]] added to graph [tinkergraph[vertices:2 edges:0]] Vertex [v[2]] property [vp[name->null]] change to [name1] in graph [tinkergraph[vertices:2 edges:0]] Vertex [v[2]] property [vp[name->null]] change to [name2] in graph [tinkergraph[vertices:2 edges:0]] Vertex [v[2]] property [vp[name->null]] change to [name2] in graph [tinkergraph[vertices:2 edges:0]] Vertex [v[3]] added to graph [tinkergraph[vertices:3 edges:0]] Vertex [v[3]] property [vp[name->null]] change to [name1] in graph [tinkergraph[vertices:3 edges:0]] Vertex [v[3]] property [vp[name->null]] change to [name2] in graph [tinkergraph[vertices:3 edges:0]] Vertex [v[3]] property [vp[name->name2]] change to [name2] in graph [tinkergraph[vertices:3 edges:0]] ``` It's not quite right as the old property will always be `vp[name->null]` when `Cardinality.list` is used or when `Cardinality.set` is used and the value doesn't exist yet. However, I think that's the only to make it work without any major breaking changes. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[tinkerpop] 01/01: TINKERPOP-2159 Fixed multi-valued property handling in EventStrategy / AddPropertyStep.
This is an automated email from the ASF dual-hosted git repository.
dkuppitz pushed a commit to branch TINKERPOP-2159
in repository https://gitbox.apache.org/repos/asf/tinkerpop.git
commit 14948cb38181a59cab0c838bd449ddb21503e1e0
Author: Daniel Kuppitz
AuthorDate: Mon Feb 18 07:30:48 2019 -0700
TINKERPOP-2159 Fixed multi-valued property handling in EventStrategy /
AddPropertyStep.
---
.../traversal/step/sideEffect/AddPropertyStep.java | 77 +++---
.../tinkergraph/structure/TinkerGraphPlayTest.java | 33 ++
2 files changed, 74 insertions(+), 36 deletions(-)
diff --git
a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/AddPropertyStep.java
b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/AddPropertyStep.java
index 7509f86..27ffe4e 100644
---
a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/AddPropertyStep.java
+++
b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/AddPropertyStep.java
@@ -41,7 +41,9 @@ import
org.apache.tinkerpop.gremlin.structure.util.detached.DetachedFactory;
import org.apache.tinkerpop.gremlin.structure.util.detached.DetachedProperty;
import
org.apache.tinkerpop.gremlin.structure.util.detached.DetachedVertexProperty;
+import java.util.Iterator;
import java.util.List;
+import java.util.Objects;
import java.util.Set;
/**
@@ -93,29 +95,58 @@ public final class AddPropertyStep
extends SideEffectStep
final Element element = traverser.get();
if (this.callbackRegistry != null) {
-final EventStrategy eventStrategy =
getTraversal().getStrategies().getStrategy(EventStrategy.class).get();
-final Property currentProperty = traverser.get().property(key);
-final boolean newProperty = element instanceof Vertex ?
currentProperty == VertexProperty.empty() : currentProperty == Property.empty();
-final Event.ElementPropertyChangedEvent evt;
-if (element instanceof Vertex)
-evt = new
Event.VertexPropertyChangedEvent(eventStrategy.detach((Vertex) element),
-newProperty ?
-eventStrategy.empty(element, key) :
-eventStrategy.detach((VertexProperty)
currentProperty), value, vertexPropertyKeyValues);
-else if (element instanceof Edge)
-evt = new
Event.EdgePropertyChangedEvent(eventStrategy.detach((Edge) element),
-newProperty ?
-eventStrategy.empty(element, key) :
-eventStrategy.detach(currentProperty), value);
-else if (element instanceof VertexProperty)
-evt = new
Event.VertexPropertyPropertyChangedEvent(eventStrategy.detach((VertexProperty)
element),
-newProperty ?
-eventStrategy.empty(element, key) :
-eventStrategy.detach(currentProperty), value);
-else
-throw new IllegalStateException(String.format("The incoming
object cannot be processed by change eventing in %s: %s",
AddPropertyStep.class.getName(), element));
-
-this.callbackRegistry.getCallbacks().forEach(c -> c.accept(evt));
+getTraversal().getStrategies().getStrategy(EventStrategy.class)
+.ifPresent(eventStrategy -> {
+Event.ElementPropertyChangedEvent evt = null;
+if (element instanceof Vertex) {
+final VertexProperty.Cardinality cardinality =
this.cardinality != null
+? this.cardinality
+:
element.graph().features().vertex().getCardinality(key);
+
+if (cardinality ==
VertexProperty.Cardinality.list) {
+evt = new
Event.VertexPropertyChangedEvent(eventStrategy.detach((Vertex) element),
+eventStrategy.empty(element, key),
value, vertexPropertyKeyValues);
+}
+else if (cardinality ==
VertexProperty.Cardinality.set) {
+Property currentProperty =
VertexProperty.empty();
+final Iterator properties
= traverser.get().properties(key);
+while (properties.hasNext()) {
+final Property property =
properties.next();
+if (Objects.equals(property.value(),
value)) {
+currentProperty = property;
+break;
+}
+}
+evt =
[tinkerpop] branch TINKERPOP-2159 created (now 14948cb)
This is an automated email from the ASF dual-hosted git repository. dkuppitz pushed a change to branch TINKERPOP-2159 in repository https://gitbox.apache.org/repos/asf/tinkerpop.git. at 14948cb TINKERPOP-2159 Fixed multi-valued property handling in EventStrategy / AddPropertyStep. This branch includes the following new commits: new 14948cb TINKERPOP-2159 Fixed multi-valued property handling in EventStrategy / AddPropertyStep. The 1 revisions listed above as "new" are entirely new to this repository and will be described in separate emails. The revisions listed as "add" were already present in the repository and have only been added to this reference.
[GitHub] spmallette commented on a change in pull request #1062: TINKERPOP-2163 Improved performance of JavaTranslator method selection
spmallette commented on a change in pull request #1062: TINKERPOP-2163 Improved
performance of JavaTranslator method selection
URL: https://github.com/apache/tinkerpop/pull/1062#discussion_r257676461
##
File path:
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/jsr223/JavaTranslator.java
##
@@ -273,4 +274,20 @@ private Method getStartMethodFromAnonymousTraversal() {
return null;
}
+
+private static final class ReflectedMethod {
+private final Method method;
+private final Parameter[] parameters;
+private final boolean hasVarArgs;
+
+public ReflectedMethod(final Method m) {
+this.method = m;
+
+// the reflection getParameters() method calls clone() every time
to get the Parameter array. caching it
+// saves a lot of extra processing
+this.parameters = m.getParameters();
+
+this.hasVarArgs = parameters.length > 0 &&
parameters[method.getParameters().length - 1].isVarArgs();
Review comment:
thanks. i should not have missed that. i'll change it and re-run the
benchmark.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[tinkerpop] branch write-TINKERPOP-2161-1 created (now eab3420)
This is an automated email from the ASF dual-hosted git repository. jorgebg pushed a change to branch write-TINKERPOP-2161-1 in repository https://gitbox.apache.org/repos/asf/tinkerpop.git. at eab3420 Use unmodifiableBuffers instead of composites This branch includes the following new commits: new eab3420 Use unmodifiableBuffers instead of composites The 1 revisions listed above as "new" are entirely new to this repository and will be described in separate emails. The revisions listed as "add" were already present in the repository and have only been added to this reference.
[tinkerpop] 01/01: Use unmodifiableBuffers instead of composites
This is an automated email from the ASF dual-hosted git repository.
jorgebg pushed a commit to branch write-TINKERPOP-2161-1
in repository https://gitbox.apache.org/repos/asf/tinkerpop.git
commit eab3420436da3d70e6ab54b79375e97594153f90
Author: Jorge Bay Gondra
AuthorDate: Mon Feb 18 11:55:53 2019 +0100
Use unmodifiableBuffers instead of composites
---
.../driver/ser/GraphBinaryMessageSerializerV1.java | 10 ++---
.../driver/ser/binary/GraphBinaryWriter.java | 16 ---
.../ser/binary/RequestMessageSerializer.java | 24 ++
.../ser/binary/ResponseMessageSerializer.java | 6 ++-
.../ser/binary/types/BigDecimalSerializer.java | 9 ++--
.../driver/ser/binary/types/BindingSerializer.java | 9 ++--
.../driver/ser/binary/types/BulkSetSerializer.java | 11 +++--
.../ser/binary/types/ByteCodeSerializer.java | 48 +---
.../ser/binary/types/CollectionSerializer.java | 9 ++--
.../driver/ser/binary/types/EdgeSerializer.java| 21 +
.../driver/ser/binary/types/GraphSerializer.java | 52 +++---
.../driver/ser/binary/types/LambdaSerializer.java | 10 ++---
.../ser/binary/types/LocalDateTimeSerializer.java | 8 ++--
.../driver/ser/binary/types/MapSerializer.java | 10 ++---
.../driver/ser/binary/types/MetricsSerializer.java | 16 +++
.../ser/binary/types/OffsetDateTimeSerializer.java | 8 ++--
.../ser/binary/types/OffsetTimeSerializer.java | 8 ++--
.../driver/ser/binary/types/PSerializer.java | 11 +++--
.../driver/ser/binary/types/PathSerializer.java| 8 ++--
.../ser/binary/types/PropertySerializer.java | 11 +++--
.../ser/binary/types/SimpleTypeSerializer.java | 34 +-
.../binary/types/TraversalMetricsSerializer.java | 9 ++--
.../binary/types/TraversalStrategySerializer.java | 10 ++---
.../ser/binary/types/TraverserSerializer.java | 9 ++--
.../driver/ser/binary/types/TreeSerializer.java| 13 +++---
.../ser/binary/types/VertexPropertySerializer.java | 15 +++
.../driver/ser/binary/types/VertexSerializer.java | 13 +++---
.../ser/binary/types/ZonedDateTimeSerializer.java | 9 ++--
.../binary/GraphBinaryMessageSerializerV1Test.java | 31 -
.../GraphBinaryReaderWriterRoundTripTest.java | 15 ++-
.../ser/binary/types/CharSerializerTest.java | 28 +++-
.../types/sample/SamplePersonSerializerTest.java | 10 -
.../driver/GraphBinaryReaderWriterBenchmark.java | 4 ++
.../gremlin/driver/GraphSONMapperBenchmark.java| 4 ++
34 files changed, 318 insertions(+), 191 deletions(-)
diff --git
a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ser/GraphBinaryMessageSerializerV1.java
b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ser/GraphBinaryMessageSerializerV1.java
index c0defd4..be916b4 100644
---
a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ser/GraphBinaryMessageSerializerV1.java
+++
b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ser/GraphBinaryMessageSerializerV1.java
@@ -20,7 +20,7 @@ package org.apache.tinkerpop.gremlin.driver.ser;
import io.netty.buffer.ByteBuf;
import io.netty.buffer.ByteBufAllocator;
-import io.netty.buffer.CompositeByteBuf;
+import io.netty.buffer.Unpooled;
import org.apache.tinkerpop.gremlin.driver.message.RequestMessage;
import org.apache.tinkerpop.gremlin.driver.message.ResponseMessage;
import org.apache.tinkerpop.gremlin.driver.ser.binary.GraphBinaryIo;
@@ -125,11 +125,9 @@ public class GraphBinaryMessageSerializerV1 extends
AbstractMessageSerializer {
@Override
public ByteBuf serializeRequestAsBinary(final RequestMessage
requestMessage, final ByteBufAllocator allocator) throws SerializationException
{
-final CompositeByteBuf result = allocator.compositeBuffer(3);
-result.addComponent(true,
allocator.buffer(1).writeByte(HEADER.length));
-result.addComponent(true,
allocator.buffer(HEADER.length).writeBytes(HEADER));
-result.addComponent(true, requestSerializer.writeValue(requestMessage,
allocator, writer));
-return result;
+return Unpooled.unmodifiableBuffer(
+allocator.buffer(1 +
HEADER.length).writeByte(HEADER.length).writeBytes(HEADER),
+requestSerializer.writeValue(requestMessage, allocator, writer));
}
@Override
diff --git
a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ser/binary/GraphBinaryWriter.java
b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ser/binary/GraphBinaryWriter.java
index 1703658..9ff773a 100644
---
a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ser/binary/GraphBinaryWriter.java
+++
b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ser/binary/GraphBinaryWriter.java
@@ -21,6 +21,7 @@ package org.apache.tinkerpop.gremlin.driver.ser.binary;
import io.netty.buffer.ByteBuf;
import
