[ 
https://issues.apache.org/jira/browse/TINKERPOP-1682?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16047997#comment-16047997
 ] 

ASF GitHub Bot commented on TINKERPOP-1682:
-------------------------------------------

Github user okram commented on a diff in the pull request:

    https://github.com/apache/tinkerpop/pull/621#discussion_r121703249
  
    --- Diff: 
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/OrderGlobalStep.java
 ---
    @@ -99,6 +99,19 @@ public void modulateBy(final Traversal.Admin<?, ?> 
traversal, final Comparator c
         }
     
         @Override
    +    public void replaceLocalChild(final Traversal.Admin<?, ?> 
oldTraversal, final Traversal.Admin<?, ?> newTraversal) {
    +        int i = 0;
    +        for (final Pair<Traversal.Admin<S, C>, Comparator<C>> pair : 
this.comparators) {
    +            final Traversal.Admin<S, C> traversal = pair.getValue0();
    +            if (null != traversal && traversal.equals(oldTraversal)) {
    +                this.comparators.set(i, 
Pair.with(this.integrateChild(newTraversal), pair.getValue1()));
    --- End diff --
    
    I would add a test case to `OrderTest` (if it doesn't already exist) that 
verifies that your strategy works as `OrderXXXStep` is complicated with how it 
deals with comparators. Thus, add a `...order().by(label())...`-style test so 
we know all is good (in the proverbial hood).


> by-modulator optimization strategy
> ----------------------------------
>
>                 Key: TINKERPOP-1682
>                 URL: https://issues.apache.org/jira/browse/TINKERPOP-1682
>             Project: TinkerPop
>          Issue Type: Improvement
>          Components: process
>    Affects Versions: 3.2.4
>            Reporter: Daniel Kuppitz
>            Assignee: Daniel Kuppitz
>
> The can be a significant difference in performance depending on whether you 
> use {{by(id)}} or {{by(id())}}:
> {noformat}
> gremlin> g.V().out().in().path().by(id()).profile()
> ==>Traversal Metrics
> Step                                                               Count  
> Traversers       Time (ms)    % Dur
> =============================================================================================================
> TinkerGraphStep(vertex,[])                                             6      
>      6          12.137    58.03
> VertexStep(OUT,vertex)                                                 6      
>      6           1.345     6.43
> VertexStep(IN,vertex)                                                 12      
>     12           0.861     4.12
> PathStep([[IdStep, ProfileStep]])                                     12      
>     12           6.570    31.42
>   IdStep                                                              36      
>     36           0.452
>                                             >TOTAL                     -      
>      -          20.916        -
> gremlin> g.V().out().in().path().by(id).profile()
> ==>Traversal Metrics
> Step                                                               Count  
> Traversers       Time (ms)    % Dur
> =============================================================================================================
> TinkerGraphStep(vertex,[])                                             6      
>      6           0.876    11.54
> VertexStep(OUT,vertex)                                                 6      
>      6           0.714     9.40
> VertexStep(IN,vertex)                                                 12      
>     12           3.299    43.43
> PathStep([id])                                                        12      
>     12           2.706    35.63
>                                             >TOTAL                     -      
>      -           7.596        -
> {noformat}
> We should have a strategy that rewrites these simple patterns. This affects 
> {{id()}}, {{label()}}, {{key()}}, {{value()}}, but also 
> {{by(values("name"))}}, which can be rewritten as {{by("name")}}



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

Reply via email to