You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tinkerpop.apache.org by okram <gi...@git.apache.org> on 2016/06/28 16:46:51 UTC
[GitHub] tinkerpop pull request #349: TINKERPOP-1349: RepeatUnrollStrategy should unr...
GitHub user okram opened a pull request:
https://github.com/apache/tinkerpop/pull/349
TINKERPOP-1349: RepeatUnrollStrategy should unroll loops while maintaining equivalent semantics.
https://issues.apache.org/jira/browse/TINKERPOP-1349
`RepeatUnrollStrategy` is a Standard-only strategy that will unroll a `repeat()` into a linear form if and only if it has a known loop amount (e.g. `times(2)`). That is `repeat(out()).times(2)` becomes `out().barrier().out().barrier()`. The `barrier()` insertions are necessary as `repeat()` itself is a barrier step and also, the benefit of bulking is leveraged. `RepeatUnrollStrategy` removes the need for `RepeatStep`, `RepeatEndStep`, and `LoopTraversal`.
`RepeatUnrollStrategyTest` ensures that unrolling is faster than not-unrolling. Here are some runtimes. The first item in the Pair is the runtime and the second item is the amount of unique objects being processed. With more unique objects, the faster it gets. Note that at 10 unique elements (tiny traversal), sometimes rolled is faster than unrolled and vice versa (i.e. they are equal).
```
rolled:[0.22843720299999998, 10] unrolled:[0.174451055, 10]
rolled:[0.514145509, 100] unrolled:[0.212532769, 100]
rolled:[1.4944357259999999, 1000] unrolled:[0.441714987, 1000]
rolled:[28.502658626, 10000] unrolled:[12.936112649999998, 10000]
```
Also in this ticket, I fixed a bug in `BranchStep` where children were not being integrated and thus, susceptible to clone-problems. Moreover, I added a timing test to `IdentityRemovalStrategyTest`. @spmallette, do you know how to make a test run once and only once in a `Parameterized` test case? If so, can you update `IdentityRemoveStrategyTest` and `RepeatUnrollStrategyTest` accordingly?
---
```
CHANGELOG
* Added `RepeatUnrollStrategy` to linearize a `repeat()`-traversal if loop amount is known at compile time.
* Fixed a bug in `BranchStep` around child integration during `clone()`.
```
VOTE +1.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/apache/tinkerpop TINKERPOP-1349
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/tinkerpop/pull/349.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 #349
----
commit 8753366a975101ba4edaa1888163abda3238f7ff
Author: Marko A. Rodriguez <ok...@gmail.com>
Date: 2016-06-28T16:44:32Z
added RepeatUnrollStrategy for when the loop amount is known at compile time. Fixed a bug in BranchStep around child integration.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tinkerpop pull request #349: TINKERPOP-1349: RepeatUnrollStrategy should unr...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/tinkerpop/pull/349
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tinkerpop issue #349: TINKERPOP-1349: RepeatUnrollStrategy should unroll loo...
Posted by okram <gi...@git.apache.org>.
Github user okram commented on the issue:
https://github.com/apache/tinkerpop/pull/349
NOTE: This can not go into `tp31/` because `NoOpBarrierStep` is new to TinkerPop 3.2.0.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tinkerpop issue #349: TINKERPOP-1349: RepeatUnrollStrategy should unroll loo...
Posted by dkuppitz <gi...@git.apache.org>.
Github user dkuppitz commented on the issue:
https://github.com/apache/tinkerpop/pull/349
VOTE: +1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tinkerpop issue #349: TINKERPOP-1349: RepeatUnrollStrategy should unroll loo...
Posted by spmallette <gi...@git.apache.org>.
Github user spmallette commented on the issue:
https://github.com/apache/tinkerpop/pull/349
All tests pass with `docker/build.sh -t -n -i`
VOTE +1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tinkerpop issue #349: TINKERPOP-1349: RepeatUnrollStrategy should unroll loo...
Posted by dkuppitz <gi...@git.apache.org>.
Github user dkuppitz commented on the issue:
https://github.com/apache/tinkerpop/pull/349
Same here, `docker/build.sh -t -i -n` reported success over night. All changes after my first vote were included in this test run.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tinkerpop issue #349: TINKERPOP-1349: RepeatUnrollStrategy should unroll loo...
Posted by spmallette <gi...@git.apache.org>.
Github user spmallette commented on the issue:
https://github.com/apache/tinkerpop/pull/349
Travis is failing - looks like an issue from Rat - missing an Apache header in `TraversalStrategyPerformanceTest`.
@okram I don't think there's a way to get a Parameterized test to run a single test once. It pretty much treats the whole test case as parameterized when you use that feature. Using `Enclosed` as you did is the only way to make that work afaik.
@pietermartin do you intend to vote on this one? you had a fair number of comments/questions on the JIRA ticket....
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---