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

Stephen Mallette commented on TINKERPOP-2393:
---------------------------------------------

> For now, we are thinking that when we replace PropertiesStep with 
> BulkPropertiesStep we can add NoOpBarrierStep too with a certain barrier 
> size, so we want BranchStep to check NoOpBarrierStep's size and to tweak how 
> many solutions it passes to downstream. 

This sounds like a reasonable solution that works within the parameters of the 
current design.

> As long term solution, however, I think BulkPropertiesStep itself should 
> implement Barrier for simplicity sake. 

I suppose that the retrieval of properties could behave as a {{Barrier}}, 
though I'm not sure what all the implications would be offhand. I think that 
the goal here would be to figure out a design that just makes this point 
extensible and easier to implement than a custom {{BulkPropertiesStep}} with a 
{{NoOpBarrier}} as you plan to do above. I assume that {{PropertiesStep}} as a 
{{Barrier}} will not cause any trouble but I do think that implementing 
{{Barrier}} there will require some special care because it may alter 
{{TraversalStrategy}} evaluations considerably as we often check for 
{{Barrier}} steps which means that this sort of change could introduce some 
breaks in functionality that we can't quite conceive yet. Anyway, interesting 
idea.



> BranchStep should respect the barrier size of BarrierStep when calling 
> applyCurrentTraverser
> --------------------------------------------------------------------------------------------
>
>                 Key: TINKERPOP-2393
>                 URL: https://issues.apache.org/jira/browse/TINKERPOP-2393
>             Project: TinkerPop
>          Issue Type: Improvement
>          Components: process
>    Affects Versions: 3.4.7
>            Reporter: redtree
>            Priority: Minor
>
> Right now in BranchStep, if children contain a BarrierStep it keeps calling 
> applyCurrentTraverser until this.starts is exhausted.
> [https://github.com/apache/tinkerpop/blob/534746b6019d0775e628375a3dfeba8a18df90fe/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/branch/BranchStep.java#L139-L140]
> This means the child step may hold all upstream solutions in memory at once, 
> and potentially memory usage is bloated.
> I think here we can change so that BarrierStep can have a method like 
> getMaxBarrierSize, and BranchStep examines the size of barrier that child 
> steps can handle at once then break the while loop either when 
> `this.starts.hasNext` becomes false or we exceed the barrier size to save the 
> potential memory consumption.
>  
> When there are multiple barrier steps as children, we may pick either max or 
> min among them.
> I am happy to implement this, but please let me know if you have any comments 
> or concerns on this change. 
> Thanks



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Reply via email to