[
https://issues.apache.org/jira/browse/TINKERPOP-2924?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17712417#comment-17712417
]
ASF GitHub Bot commented on TINKERPOP-2924:
-------------------------------------------
porunov commented on code in PR #2022:
URL: https://github.com/apache/tinkerpop/pull/2022#discussion_r1166916791
##########
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/PropertyMapStep.java:
##########
@@ -216,7 +177,75 @@ public int getIncludedTokens() {
return this.tokens;
}
- private boolean includeToken(final int token) {
+ protected boolean includeToken(final int token) {
return 0 != (this.tokens & token);
}
+
+ protected void addIncludedOptions(Element element, Map<Object, Object>
map){
+ if (this.returnType == PropertyType.VALUE) {
+ if (includeToken(WithOptions.ids)) map.put(T.id,
getElementId(element));
+ if (element instanceof VertexProperty) {
+
+ if (includeToken(WithOptions.keys)) map.put(T.key,
getVertexPropertyKey((VertexProperty<?>) element));
+ if (includeToken(WithOptions.values)) map.put(T.value,
getVertexPropertyValue((VertexProperty<?>) element));
+ } else {
+ if (includeToken(WithOptions.labels)) map.put(T.label,
getElementLabel(element));
+ }
+ }
+ }
+
+ protected Object getElementId(Element element){
+ return element.id();
+ }
+
+ protected String getElementLabel(Element element){
+ return element.label();
+ }
+
+ protected String getVertexPropertyKey(VertexProperty<?> vertexProperty){
+ return vertexProperty.key();
+ }
+
+ protected Object getVertexPropertyValue(VertexProperty<?> vertexProperty){
+ return vertexProperty.value();
+ }
+
+ protected void addElementProperties(final Traverser.Admin<Element>
traverser, Map<Object, Object> map){
+ final Element element = traverser.get();
+ final boolean isVertex = element instanceof Vertex;
+
+ final Iterator<? extends Property> properties = null ==
this.propertyTraversal ?
+ element.properties(this.propertyKeys) :
+ TraversalUtil.applyAll(traverser, this.propertyTraversal);
+
+ while (properties.hasNext()) {
+ final Property<?> property = properties.next();
+ final Object value = this.returnType == PropertyType.VALUE ?
property.value() : property;
+ if (isVertex) {
+ map.compute(property.key(), (k, v) -> {
+ final List<Object> values = v != null ? (List<Object>) v :
new ArrayList<>();
+ values.add(value);
+ return values;
+ });
+ } else {
+ map.put(property.key(), value);
+ }
+ }
+ }
+
+ protected void applyTraversalRingToMap(Map<Object, Object> map){
+ if (!traversalRing.isEmpty()) {
+ // will cop a ConcurrentModification if a key is dropped so need
this little copy here
+ final List<Object> keys = new ArrayList<>(map.keySet());
Review Comment:
The only logic change was here. Changed the structure from HashSet to
ArrayList as HashSet copy will take N*logN complexity but ArrayList copy will
take N complexity only. Moreover, the copied underlying HashSet array is 25%
bigger than the underlying array of ArrayList because HashSet needs a bigger
array to have smaller collision chance. In contrariety ArrayList won't have any
collisions.
I.e. ArrayList here will have smaller overhead and faster insertion. We
already guaranteed keys uniqueness during this list traversal by copying data
from `map.keySet()`. No need to ensure uniqueness again here for below loop
operation.
> Refactor PropertyMapStep to be able to overwrite map method
> -----------------------------------------------------------
>
> Key: TINKERPOP-2924
> URL: https://issues.apache.org/jira/browse/TINKERPOP-2924
> Project: TinkerPop
> Issue Type: Improvement
> Components: driver
> Affects Versions: 3.6.2
> Reporter: Oleksandr Porunov
> Priority: Minor
>
> We would like to extend `PropertyMapStep` and overwrite some of it's
> functionality (in `map` method), so that we could leverage multi-query
> optimization in JanusGraph for `PropertyMapStep`.
> Unfortunately, some of it's utility methods and fields are `private`. Thus,
> we should duplicate `includeToken` logic and we don't have any access to
> `traversalRing` because it's private and is created inside the constructor.
> I would suggest making all of those private fields / methods as protected
> (similarly as it's done in `PropertiesStep`), so that there would be a
> possibility for overwrite without logic duplication.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
