tinkerpop-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From spmallette <...@git.apache.org>
Subject [GitHub] tinkerpop pull request #838: TINKERPOP-1822: Change default RepeatStep to DF...
Date Wed, 06 Jun 2018 12:22:09 GMT
Github user spmallette commented on a diff in the pull request:

    https://github.com/apache/tinkerpop/pull/838#discussion_r193389532
  
    --- Diff: gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/branch/RepeatStep.java
---
    @@ -273,11 +300,40 @@ public RepeatEndStep(final Traversal.Admin traversal) {
                 super(traversal);
             }
     
    +        final LinkedList<Traverser.Admin<S>> stashedStarts = new LinkedList<>();
    --- End diff --
    
    An `ArrayList` sounds reasonable to me as far as the right `List` implementation for how
`stashedStarts` is being used. When I'd posed the question I was more thinking about the more
general increased memory requirements for doing DFS in this fashion as we basically have to
accumulate what could be a fairly large list in memory in order to perform this operation.
I suppose that we do such things in `fold()`-ing steps but the user is explicitly aware of
their choice to do that when they use such a step. In the case of `stashedStarts` the memory
requirement for choosing DFS is somewhat hidden as it's not clear from the Gremlin they've
written that an internal list is being built. Perhaps that's simply a side-effect of allowing
this to work in the way that it does (as you alluded to in your comment).
    
    I'm still thinking that DFS will be something that users will invoke in specific use cases
where they will be more aware of the consequences of what it is that they are doing. If that
is the case, then this would perhaps just be one more consequence of making that choice to
consider.
    
    I'd be curious to see some JFRs around the different modes of execution that we now have.
Perhaps some microbenchmarks are in order too. And then the fun part....does OLAP still work
without any change? :smile: 


---

Mime
View raw message