[jira] [Commented] (TINKERPOP-1471) IncidentToAdjacentStrategy use hidden marker to avoid repeated recursion.

[ASF GitHub Bot (JIRA)]

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

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

Github user asfgit closed the pull request at:

https://github.com/apache/tinkerpop/pull/474


> IncidentToAdjacentStrategy use hidden marker to avoid repeated recursion.
> -
>
> Key: TINKERPOP-1471
> URL: https://issues.apache.org/jira/browse/TINKERPOP-1471
> Project: TinkerPop
>  Issue Type: Improvement
>  Components: process
>Affects Versions: 3.2.2
>Reporter: Marko A. Rodriguez
>Assignee: Daniel Kuppitz
>
> TINKERPOP-1456 introduces the notion that "hidden labels" on steps will be 
> removed from the traversal prior to evaluation. This use of "hidden labels" 
> was necessary for {{SubgraphStrategy}} and it is not always possible to 
> remove the add "hidden labels" in {{SubgraphStrategy}} completely. Thus, 
> prior to evaluation, in hidden labels are removed.
> This concept of "hidden labels" is useful for marking steps in a traversal so 
> that  a strategy applied at a child gets information from the strategy 
> applied at a parent.
> This can be used to make {{IncidentToAdjacentStrategy}} faster. In 
> particular, the following recursion is called repeatedly for each child.
> {code}
>   if 
> (TraversalHelper.hasStepOfAssignableClassRecursively(INVALIDATING_STEP_CLASSES,
>  TraversalHelper.getRootTraversal(traversal)))
> return;
> {code}
> Instead, it would be smart to do this and then, you don't need to recurse 
> from the root traversal over and over again.
> {code}
> private final void addMarkerToChildren(final Traversal.Admin 
> traversal) {
> traversal.getStartStep().addLabel(MARKER);
> for (final Step step : traversal.getSteps()) {
> if (step instanceof TraversalParent) {
> ((TraversalParent) 
> step).getLocalChildren().forEach(this::addMarkerToChildren);
> ((TraversalParent) 
> step).getGlobalChildren().forEach(this::addMarkerToChildren);
> }
> }
> }
> public void apply(Traversal traversal) {
>   if(traversal.getParent() instanceof EmptyStep) {
> boolean mark = 
> TraversalHelper.hasStepOfAssignableClassRecursively(INVALIDATING_STEP_CLASSES,
>  traversal)
> if(mark)  this.addMarkerToChildren(traversal)
>   }
>   // if the marker exists, don't optimize
>   if(traversal.getStartStep().getLabels().contains(MARKER)) {
> traversal.getStartStep().removeLabel(MARKER);
> return;
>   }
> // else do the rest of the code.
> }
> {code}



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (TINKERPOP-1471) IncidentToAdjacentStrategy use hidden marker to avoid repeated recursion.

[ASF GitHub Bot (JIRA)]

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

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

Github user spmallette commented on the issue:

https://github.com/apache/tinkerpop/pull/474
  
All tests pass with `docker/build.sh -t -n -i`

VOTE +1


> IncidentToAdjacentStrategy use hidden marker to avoid repeated recursion.
> -
>
> Key: TINKERPOP-1471
> URL: https://issues.apache.org/jira/browse/TINKERPOP-1471
> Project: TinkerPop
>  Issue Type: Improvement
>  Components: process
>Affects Versions: 3.2.2
>Reporter: Marko A. Rodriguez
>Assignee: Daniel Kuppitz
>
> TINKERPOP-1456 introduces the notion that "hidden labels" on steps will be 
> removed from the traversal prior to evaluation. This use of "hidden labels" 
> was necessary for {{SubgraphStrategy}} and it is not always possible to 
> remove the add "hidden labels" in {{SubgraphStrategy}} completely. Thus, 
> prior to evaluation, in hidden labels are removed.
> This concept of "hidden labels" is useful for marking steps in a traversal so 
> that  a strategy applied at a child gets information from the strategy 
> applied at a parent.
> This can be used to make {{IncidentToAdjacentStrategy}} faster. In 
> particular, the following recursion is called repeatedly for each child.
> {code}
>   if 
> (TraversalHelper.hasStepOfAssignableClassRecursively(INVALIDATING_STEP_CLASSES,
>  TraversalHelper.getRootTraversal(traversal)))
> return;
> {code}
> Instead, it would be smart to do this and then, you don't need to recurse 
> from the root traversal over and over again.
> {code}
> private final void addMarkerToChildren(final Traversal.Admin 
> traversal) {
> traversal.getStartStep().addLabel(MARKER);
> for (final Step step : traversal.getSteps()) {
> if (step instanceof TraversalParent) {
> ((TraversalParent) 
> step).getLocalChildren().forEach(this::addMarkerToChildren);
> ((TraversalParent) 
> step).getGlobalChildren().forEach(this::addMarkerToChildren);
> }
> }
> }
> public void apply(Traversal traversal) {
>   if(traversal.getParent() instanceof EmptyStep) {
> boolean mark = 
> TraversalHelper.hasStepOfAssignableClassRecursively(INVALIDATING_STEP_CLASSES,
>  traversal)
> if(mark)  this.addMarkerToChildren(traversal)
>   }
>   // if the marker exists, don't optimize
>   if(traversal.getStartStep().getLabels().contains(MARKER)) {
> traversal.getStartStep().removeLabel(MARKER);
> return;
>   }
> // else do the rest of the code.
> }
> {code}



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (TINKERPOP-1471) IncidentToAdjacentStrategy use hidden marker to avoid repeated recursion.

[ASF GitHub Bot (JIRA)]

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

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

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

https://github.com/apache/tinkerpop/pull/474#discussion_r86029705
  
--- Diff: 
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/decoration/SubgraphStrategy.java
 ---
@@ -105,21 +105,11 @@ private SubgraphStrategy(final Traversal 
vertexCriterion, final Trave
 this.vertexPropertyCriterion = null == vertexPropertyCriterion ? 
null : vertexPropertyCriterion.asAdmin().clone();
 
 if (null != this.vertexCriterion)
-this.addLabelMarker(this.vertexCriterion);
+TraversalHelper.applyTraversalRecursively(t -> 
t.getStartStep().addLabel(MARKER), this.vertexCriterion);
--- End diff --

Ha, it's getting late here.


> IncidentToAdjacentStrategy use hidden marker to avoid repeated recursion.
> -
>
> Key: TINKERPOP-1471
> URL: https://issues.apache.org/jira/browse/TINKERPOP-1471
> Project: TinkerPop
>  Issue Type: Improvement
>  Components: process
>Affects Versions: 3.2.2
>Reporter: Marko A. Rodriguez
>Assignee: Daniel Kuppitz
>
> TINKERPOP-1456 introduces the notion that "hidden labels" on steps will be 
> removed from the traversal prior to evaluation. This use of "hidden labels" 
> was necessary for {{SubgraphStrategy}} and it is not always possible to 
> remove the add "hidden labels" in {{SubgraphStrategy}} completely. Thus, 
> prior to evaluation, in hidden labels are removed.
> This concept of "hidden labels" is useful for marking steps in a traversal so 
> that  a strategy applied at a child gets information from the strategy 
> applied at a parent.
> This can be used to make {{IncidentToAdjacentStrategy}} faster. In 
> particular, the following recursion is called repeatedly for each child.
> {code}
>   if 
> (TraversalHelper.hasStepOfAssignableClassRecursively(INVALIDATING_STEP_CLASSES,
>  TraversalHelper.getRootTraversal(traversal)))
> return;
> {code}
> Instead, it would be smart to do this and then, you don't need to recurse 
> from the root traversal over and over again.
> {code}
> private final void addMarkerToChildren(final Traversal.Admin 
> traversal) {
> traversal.getStartStep().addLabel(MARKER);
> for (final Step step : traversal.getSteps()) {
> if (step instanceof TraversalParent) {
> ((TraversalParent) 
> step).getLocalChildren().forEach(this::addMarkerToChildren);
> ((TraversalParent) 
> step).getGlobalChildren().forEach(this::addMarkerToChildren);
> }
> }
> }
> public void apply(Traversal traversal) {
>   if(traversal.getParent() instanceof EmptyStep) {
> boolean mark = 
> TraversalHelper.hasStepOfAssignableClassRecursively(INVALIDATING_STEP_CLASSES,
>  traversal)
> if(mark)  this.addMarkerToChildren(traversal)
>   }
>   // if the marker exists, don't optimize
>   if(traversal.getStartStep().getLabels().contains(MARKER)) {
> traversal.getStartStep().removeLabel(MARKER);
> return;
>   }
> // else do the rest of the code.
> }
> {code}



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (TINKERPOP-1471) IncidentToAdjacentStrategy use hidden marker to avoid repeated recursion.

[ASF GitHub Bot (JIRA)]

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

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

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

https://github.com/apache/tinkerpop/pull/474#discussion_r86025437
  
--- Diff: 
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/decoration/SubgraphStrategy.java
 ---
@@ -105,21 +105,11 @@ private SubgraphStrategy(final Traversal 
vertexCriterion, final Trave
 this.vertexPropertyCriterion = null == vertexPropertyCriterion ? 
null : vertexPropertyCriterion.asAdmin().clone();
 
 if (null != this.vertexCriterion)
-this.addLabelMarker(this.vertexCriterion);
+TraversalHelper.applyTraversalRecursively(t -> 
t.getStartStep().addLabel(MARKER), this.vertexCriterion);
--- End diff --

`vertexCriterion` can be `null`. In that case 
`TraversalHelper.applyTraversalRecursively` and/or the consumer would throw a 
NPE.


> IncidentToAdjacentStrategy use hidden marker to avoid repeated recursion.
> -
>
> Key: TINKERPOP-1471
> URL: https://issues.apache.org/jira/browse/TINKERPOP-1471
> Project: TinkerPop
>  Issue Type: Improvement
>  Components: process
>Affects Versions: 3.2.2
>Reporter: Marko A. Rodriguez
>Assignee: Daniel Kuppitz
>
> TINKERPOP-1456 introduces the notion that "hidden labels" on steps will be 
> removed from the traversal prior to evaluation. This use of "hidden labels" 
> was necessary for {{SubgraphStrategy}} and it is not always possible to 
> remove the add "hidden labels" in {{SubgraphStrategy}} completely. Thus, 
> prior to evaluation, in hidden labels are removed.
> This concept of "hidden labels" is useful for marking steps in a traversal so 
> that  a strategy applied at a child gets information from the strategy 
> applied at a parent.
> This can be used to make {{IncidentToAdjacentStrategy}} faster. In 
> particular, the following recursion is called repeatedly for each child.
> {code}
>   if 
> (TraversalHelper.hasStepOfAssignableClassRecursively(INVALIDATING_STEP_CLASSES,
>  TraversalHelper.getRootTraversal(traversal)))
> return;
> {code}
> Instead, it would be smart to do this and then, you don't need to recurse 
> from the root traversal over and over again.
> {code}
> private final void addMarkerToChildren(final Traversal.Admin 
> traversal) {
> traversal.getStartStep().addLabel(MARKER);
> for (final Step step : traversal.getSteps()) {
> if (step instanceof TraversalParent) {
> ((TraversalParent) 
> step).getLocalChildren().forEach(this::addMarkerToChildren);
> ((TraversalParent) 
> step).getGlobalChildren().forEach(this::addMarkerToChildren);
> }
> }
> }
> public void apply(Traversal traversal) {
>   if(traversal.getParent() instanceof EmptyStep) {
> boolean mark = 
> TraversalHelper.hasStepOfAssignableClassRecursively(INVALIDATING_STEP_CLASSES,
>  traversal)
> if(mark)  this.addMarkerToChildren(traversal)
>   }
>   // if the marker exists, don't optimize
>   if(traversal.getStartStep().getLabels().contains(MARKER)) {
> traversal.getStartStep().removeLabel(MARKER);
> return;
>   }
> // else do the rest of the code.
> }
> {code}



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (TINKERPOP-1471) IncidentToAdjacentStrategy use hidden marker to avoid repeated recursion.

[ASF GitHub Bot (JIRA)]

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

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

Github user dkuppitz commented on the issue:

https://github.com/apache/tinkerpop/pull/474
  
VOTE: +1

A little refactoring would be nice though:

```
for (final Step step : traversal.getSteps()) {
if (step instanceof TraversalParent) {
((TraversalParent) 
step).getLocalChildren().forEach(this::addLabelMarker);
((TraversalParent) 
step).getGlobalChildren().forEach(this::addLabelMarker);
}
}
```
... should be as easy as:

```
TraversalHelper.forEachNestedTraversal(traversal, this::addLabelMarker);
```


> IncidentToAdjacentStrategy use hidden marker to avoid repeated recursion.
> -
>
> Key: TINKERPOP-1471
> URL: https://issues.apache.org/jira/browse/TINKERPOP-1471
> Project: TinkerPop
>  Issue Type: Improvement
>  Components: process
>Affects Versions: 3.2.2
>Reporter: Marko A. Rodriguez
>Assignee: Daniel Kuppitz
>
> TINKERPOP-1456 introduces the notion that "hidden labels" on steps will be 
> removed from the traversal prior to evaluation. This use of "hidden labels" 
> was necessary for {{SubgraphStrategy}} and it is not always possible to 
> remove the add "hidden labels" in {{SubgraphStrategy}} completely. Thus, 
> prior to evaluation, in hidden labels are removed.
> This concept of "hidden labels" is useful for marking steps in a traversal so 
> that  a strategy applied at a child gets information from the strategy 
> applied at a parent.
> This can be used to make {{IncidentToAdjacentStrategy}} faster. In 
> particular, the following recursion is called repeatedly for each child.
> {code}
>   if 
> (TraversalHelper.hasStepOfAssignableClassRecursively(INVALIDATING_STEP_CLASSES,
>  TraversalHelper.getRootTraversal(traversal)))
> return;
> {code}
> Instead, it would be smart to do this and then, you don't need to recurse 
> from the root traversal over and over again.
> {code}
> private final void addMarkerToChildren(final Traversal.Admin 
> traversal) {
> traversal.getStartStep().addLabel(MARKER);
> for (final Step step : traversal.getSteps()) {
> if (step instanceof TraversalParent) {
> ((TraversalParent) 
> step).getLocalChildren().forEach(this::addMarkerToChildren);
> ((TraversalParent) 
> step).getGlobalChildren().forEach(this::addMarkerToChildren);
> }
> }
> }
> public void apply(Traversal traversal) {
>   if(traversal.getParent() instanceof EmptyStep) {
> boolean mark = 
> TraversalHelper.hasStepOfAssignableClassRecursively(INVALIDATING_STEP_CLASSES,
>  traversal)
> if(mark)  this.addMarkerToChildren(traversal)
>   }
>   // if the marker exists, don't optimize
>   if(traversal.getStartStep().getLabels().contains(MARKER)) {
> traversal.getStartStep().removeLabel(MARKER);
> return;
>   }
> // else do the rest of the code.
> }
> {code}



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (TINKERPOP-1471) IncidentToAdjacentStrategy use hidden marker to avoid repeated recursion.

[ASF GitHub Bot (JIRA)]

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

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

GitHub user okram opened a pull request:

https://github.com/apache/tinkerpop/pull/474

TINKERPOP-1471: IncidentToAdjacentStrategy use hidden marker to avoid 
repeated recursion.

https://issues.apache.org/jira/browse/TINKERPOP-1471

Hidden labels can be used by strategies as a way to communicate between 
"layers" of the compilation tree. This model is now applied to 
`IncidentToAdjacentStrategy` where if the traversal (as a whole) should NOT be 
processed by `IncidentToAdjacentStrategy`, then each traversal in the tree is 
"marked." If the traversal can be processed by `IncidentToAdjacentStrategy`, 
then there is no need to mark the traversals. This removes the need to, for 
every traversal, do a recursion from the parent traversal looking for 
invalidating step classes.

VOTE +1.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/apache/tinkerpop TINKERPOP-1471

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/tinkerpop/pull/474.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #474


commit 6a1a0741ea8ee64427a126e8980ecf7db1a3299a
Author: Marko A. Rodriguez 
Date:   2016-11-01T17:54:50Z

IncidentToAdjacentStrategy now uses a hidden marker label to remove the 
need for constant rootTraversal recrussion in search of invalidating steps.




> IncidentToAdjacentStrategy use hidden marker to avoid repeated recursion.
> -
>
> Key: TINKERPOP-1471
> URL: https://issues.apache.org/jira/browse/TINKERPOP-1471
> Project: TinkerPop
>  Issue Type: Improvement
>  Components: process
>Affects Versions: 3.2.2
>Reporter: Marko A. Rodriguez
>Assignee: Daniel Kuppitz
>
> TINKERPOP-1456 introduces the notion that "hidden labels" on steps will be 
> removed from the traversal prior to evaluation. This use of "hidden labels" 
> was necessary for {{SubgraphStrategy}} and it is not always possible to 
> remove the add "hidden labels" in {{SubgraphStrategy}} completely. Thus, 
> prior to evaluation, in hidden labels are removed.
> This concept of "hidden labels" is useful for marking steps in a traversal so 
> that  a strategy applied at a child gets information from the strategy 
> applied at a parent.
> This can be used to make {{IncidentToAdjacentStrategy}} faster. In 
> particular, the following recursion is called repeatedly for each child.
> {code}
>   if 
> (TraversalHelper.hasStepOfAssignableClassRecursively(INVALIDATING_STEP_CLASSES,
>  TraversalHelper.getRootTraversal(traversal)))
> return;
> {code}
> Instead, it would be smart to do this and then, you don't need to recurse 
> from the root traversal over and over again.
> {code}
> private final void addMarkerToChildren(final Traversal.Admin 
> traversal) {
> traversal.getStartStep().addLabel(MARKER);
> for (final Step step : traversal.getSteps()) {
> if (step instanceof TraversalParent) {
> ((TraversalParent) 
> step).getLocalChildren().forEach(this::addMarkerToChildren);
> ((TraversalParent) 
> step).getGlobalChildren().forEach(this::addMarkerToChildren);
> }
> }
> }
> public void apply(Traversal traversal) {
>   if(traversal.getParent() instanceof EmptyStep) {
> boolean mark = 
> TraversalHelper.hasStepOfAssignableClassRecursively(INVALIDATING_STEP_CLASSES,
>  traversal)
> if(mark)  this.addMarkerToChildren(traversal)
>   }
>   // if the marker exists, don't optimize
>   if(traversal.getStartStep().getLabels().contains(MARKER)) {
> traversal.getStartStep().removeLabel(MARKER);
> return;
>   }
> // else do the rest of the code.
> }
> {code}



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)