-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Performance issue on vertex cache with Guava implementation #3185
Comments
Thanks for reporting @ministat! Seems a duplicate of #871 (which was created by the author of Guava, interesting!). Usually, we close newer issues if they are duplicates, but your discovery and experiments are very valuable so I'd like to keep both issues open. Would you like to contribute your implementation back to the community by submitting a PR? That would be very helpful! |
Yes, I have submitted the PR. But I'm blocked by CLA.
|
Identified 8 Guava caches in JanusGraph. Posted them here: #871 (comment) |
Fix the issue JanusGraph#3185 Signed-off-by: Hongjiang Zhang <[email protected]>
Fix the issue JanusGraph#3185 Signed-off-by: Hongjiang Zhang <[email protected]>
Fix the issue JanusGraph#3185 Signed-off-by: Hongjiang Zhang <[email protected]>
Fix the issue JanusGraph#3185 Signed-off-by: Hongjiang Zhang <[email protected]>
Fix the issue JanusGraph#3185 Signed-off-by: Hongjiang Zhang <[email protected]>
Fix the issue JanusGraph#3185 Signed-off-by: Hongjiang Zhang <[email protected]>
Fix the issue JanusGraph#3185 Signed-off-by: Hongjiang Zhang <[email protected]>
Fix the issue #3185 Signed-off-by: Hongjiang Zhang <[email protected]>
This is a follow-up commit to commit 9a22a1a Move the rest (7) caches from Guava implementation to Caffeine implementation This commit also removes a previously added Benchmark class with the obsolate GuavaVertexCache.class. The benchmark is provided in the next GitHub PR: JanusGraph#3188 which shows that Caffeine has better performance characteristics in all scenarious. ConcurencyLevel is not provided for cache implementation. The reason for this can be found by the following link https://groups.google.com/g/guava-discuss/c/BnoN9q9fDgw/m/6kF6H2aLAgAJ Related to JanusGraph#3188 and JanusGraph#3185 Partially related to JanusGraph#2033 Fixes JanusGraph#871
This is a follow-up commit to commit 9a22a1a Move the rest (7) caches from Guava implementation to Caffeine implementation This commit also removes a previously added Benchmark class with the obsolate GuavaVertexCache.class. The benchmark is provided in the next GitHub PR: JanusGraph#3188 which shows that Caffeine has better performance characteristics in all scenarious. ConcurencyLevel is not provided for cache implementation. The reason for this can be found by the following link https://groups.google.com/g/guava-discuss/c/BnoN9q9fDgw/m/6kF6H2aLAgAJ Related to JanusGraph#3188 and JanusGraph#3185 Partially related to JanusGraph#2033 Fixes JanusGraph#871 Signed-off-by: Oleksandr Porunov <[email protected]>
This is a follow-up commit to the 9a22a1a commit Migrates the rest 7 caches from Guava implementation to Caffeine implementation This commit also removes a previously added Benchmark class with the obsolate GuavaVertexCache.class. The benchmark is provided in the next GitHub PR: JanusGraph#3188 which shows that Caffeine has better performance characteristics in all scenarious. ConcurencyLevel is not provided for the cache implementation anymore. The reason for this can be found by the following link https://groups.google.com/g/guava-discuss/c/BnoN9q9fDgw/m/6kF6H2aLAgAJ Related to JanusGraph#3188 and JanusGraph#3185 Partially related to JanusGraph#2033 Fixes JanusGraph#871 Signed-off-by: Oleksandr Porunov <[email protected]>
This is a follow-up commit to the 9a22a1a commit Migrates the rest 7 caches from Guava implementation to Caffeine implementation This commit also removes a previously added Benchmark class with the obsolate GuavaVertexCache.class. The benchmark is provided in the next GitHub PR: #3188 which shows that Caffeine has better performance characteristics in all scenarious. ConcurencyLevel is not provided for the cache implementation anymore. The reason for this can be found by the following link https://groups.google.com/g/guava-discuss/c/BnoN9q9fDgw/m/6kF6H2aLAgAJ Related to #3188 and #3185 Partially related to #2033 Fixes #871 Signed-off-by: Oleksandr Porunov <[email protected]>
This is a follow-up commit to the 9a22a1a commit Migrates the rest 7 caches from Guava implementation to Caffeine implementation This commit also removes a previously added Benchmark class with the obsolate GuavaVertexCache.class. The benchmark is provided in the next GitHub PR: #3188 which shows that Caffeine has better performance characteristics in all scenarious. ConcurencyLevel is not provided for the cache implementation anymore. The reason for this can be found by the following link https://groups.google.com/g/guava-discuss/c/BnoN9q9fDgw/m/6kF6H2aLAgAJ Related to #3188 and #3185 Partially related to #2033 Fixes #871 Signed-off-by: Oleksandr Porunov <[email protected]>
JanusGraph's GuavaVertexCache will cause memory leak in 100% read and heavy read scenario, especially used in OLAP cases. The Guava cache should be replaced by Caffiene cache to fix this issue according to Guava's community feedback.
See google/guava#2408 and google/guava#2063.
In my local JanusGraph, after I replaced Guava cache with Caffeine cache, the OLAP case, for example, g.V().count() takes 18 minutes to finish the scan of a graph with 500 million vertex and 6 billion edges. Before this fix, it takes 45 minutes. So, the fix gets ~3X benefit.
For confirmed bugs, please report:
Stack Trace (if you have one)
The text was updated successfully, but these errors were encountered: