Skip to content

TINKERPOP-3100 Fix problem in recursive calls to Traversal lock() #3152

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: 3.8-dev
Choose a base branch
from

Conversation

spmallette
Copy link
Contributor

@spmallette spmallette commented Jul 3, 2025

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

Drastically improves the performance of lock() as you lose the excessive recursion and avoid having to reidentify steps repeatedly.

Benchmark                                  Mode  Cnt   Score    Error   Units
TraversalLockBenchmark.testLockTraversal  thrpt   10  ≈ 10⁻⁴           ops/ms

after

Benchmark                                  Mode  Cnt   Score   Error   Units
TraversalLockBenchmark.testLockTraversal  thrpt   10  18.784 ± 0.334  ops/ms

Messily merged this to 3.7-dev with 98ffa44 and removed the "deep" traversal tests/benchmarks because jdk8 wouldn't compile that code for some reason. works fine for jdk11/17. code is good for jdk8 when the traversal can compile so that performance fix still landed there. retargeted this PR to 3.8-dev where JDK8 is expected to be dropped and put the "deep" test/benchmarks back. This can merge after the JDK8 change goes in.

VOTE +1

@camfiander-sonrai
Copy link

VOTE +1

1 similar comment
@xiazcy
Copy link
Contributor

xiazcy commented Jul 3, 2025

VOTE +1

@Cole-Greer
Copy link
Contributor

VOTE +1 (pending actions passing)

This was done because jdk8 could not compile the deep traversal tests that were used in testing. Adding the benchmarks and tests used to develop this work back since jdk8 is not an issue for 3.8.x.
@spmallette spmallette changed the base branch from 3.7-dev to 3.8-dev July 9, 2025 12:01
@spmallette
Copy link
Contributor Author

Updated description around the build failures.......

@Cole-Greer
Copy link
Contributor

@spmallette This should be good to rebase and merge now that https://issues.apache.org/jira/browse/TINKERPOP-3172 is in.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants