Github user svanteschubert commented on the issue:
https://github.com/apache/tinkerpop/pull/892
I use to work on the console, stability and clarity comes there before the
performance when drafting on a solution (my product version would run from
the console anyway).
Do you think there is a performance real performance penalty when we do
sorting for serialization?
I am available for a code review over VOIP if you like to speed up things,
Stephen.
PS: The regarding your question, I looked it up on the GraphML Primer, it
is an internal ID, see
http://graphml.graphdrawing.org/primer/graphml-primer.html#AttributesDefinition
2018-07-16 13:45 GMT+02:00 stephen mallette <[email protected]>:
> *@spmallette* commented on this pull request.
> ------------------------------
>
> In gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/
> structure/io/graphml/GraphMLWriter.java
> <https://github.com/apache/tinkerpop/pull/892#discussion_r202653620>:
>
> > @@ -218,20 +220,33 @@ private void writeTypes(final Map<String, String>
identifiedVertexKeyTypes,
> final Map<String, String>
identifiedEdgeKeyTypes,
> final XMLStreamWriter writer) throws
XMLStreamException {
> // <key id="weight" for="edge" attr.name="weight"
attr.type="float"/>
> - final Collection<String> vertexKeySet =
getVertexKeysAndNormalizeIfRequired(identifiedVertexKeyTypes);
> + Collection<String> vertexKeySet =
getDataStructureKeysAndNormalizeIfRequired(identifiedVertexKeyTypes);
> + Collection<String> edgeKeySet =
getDataStructureKeysAndNormalizeIfRequired(identifiedEdgeKeyTypes);
> + // in case vertex and edge may have the same attribute name, the
key id in graphml have to be different
> + intersection = CollectionUtils.intersection(vertexKeySet,
edgeKeySet);
>
> In one respect, I think that using a suffix in the id is a good idea
> generally - in other words, don't bother to even track the intersections,
> just add the suffix, but I'm not sure how different tools use the id
> attribute. I assume they use it as we use it, just to look up the
> attr.name and attr.value - if so, then the id is simply internal to the
> graphml and you don't need to track for intersections to add the suffix.
>
> If that is not the case and the choice is to not generalize this, then
> we'd need tests to cover this logic.
>
> â
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
>
<https://github.com/apache/tinkerpop/pull/892#pullrequestreview-137390995>,
> or mute the thread
>
<https://github.com/notifications/unsubscribe-auth/AAyW2-CrJxLMO6tNEZK0-nwK9TZ8TIl9ks5uHHzmgaJpZM4VQWY9>
> .
>
---
