tinkerpop-dev mailing list archives

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

    https://github.com/apache/tinkerpop/pull/838#discussion_r192728027
  
    --- 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 --
    
    Sorry for the delay, things have gotten fairly busy for me. I'd given a little bit of
thought around this, but not a whole lot. I don't think this needs to be a linked list, it
could likely an ArrayList or an ArrayDeque and the entries would take up less memory. ArrayDeque
may be preferable for large graphs since it'll resize less frequently than an ArrayList, but
it will consume more memory than an ArrayList by default.
    
    However, I think having a `stashedStarts` here is necessary from the view of "one piece
of code serving 2 algorithms" If this were to be completely rewritten as DFS (I don't know
how to do that at this point, but for the sake of argument), and there was a desire to use
the same code to utilize BFS, there'd likely be a need to have stashed starts to achieve BFS.

    
    That's about as far as I've gotten in thinking about this for now.


---

Mime
View raw message