janusgraph

janusgraph copied to clipboard

This commit refactors the codebase by changing all long-type ID into Object-type in all method signatures, return types, etc. No functionality is affected, except that IDs are now stored as objects rather than primitives in the memory, which increases overhead. This is the first step to implement #1221.

This PR is to replace https://github.com/JanusGraph/janusgraph/pull/2942 because the benchmark PR commenter is only available from the JanusGraph repo.

---

Thank you for contributing to JanusGraph!

In order to streamline the review of the contribution we ask you to ensure the following steps have been taken:

For all changes:

• [ ] Is there an issue associated with this PR? Is it referenced in the commit message?
• [ ] Does your PR body contain #xyz where xyz is the issue number you are trying to resolve?
• [ ] Has your PR been rebased against the latest commit within the target branch (typically master)?
• [ ] Is your initial contribution a single, squashed commit?

For code changes:

• [ ] Have you written and/or updated unit tests to verify your changes?
• [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under ASF 2.0?
• [ ] If applicable, have you updated the LICENSE.txt file, including the main LICENSE.txt file in the root of this repository?
• [ ] If applicable, have you updated the NOTICE.txt file, including the main NOTICE.txt file found in the root of this repository?

For documentation related changes:

• [ ] Have you ensured that format looks appropriate for the output in which it is rendered?

Just did a force-commit to force benchmark CI to run again. The previous result was too good to be true, and was likely caused by variance in GitHub Actions environment itself.

FYI, previous result was:

Benchmark suite	Current: d4b1d00	Previous: 15a00b7	Ratio
org.janusgraph.MgmtOlapJobBenchmark.runRemoveIndex	114.37607429858585 ms/op	116.5591583128964 ms/op	0.98
org.janusgraph.JanusGraphSpeedBenchmark.basicAddAndDelete	12809.486658692262 ms/op	18173.307486715472 ms/op	0.70
org.janusgraph.GraphCentricQueryBenchmark.getVertices	1923.0868543083106 ms/op	2783.9976568139846 ms/op	0.69
org.janusgraph.MgmtOlapJobBenchmark.runReindex	363.0432188466667 ms/op	464.2457100388889 ms/op	0.78
org.janusgraph.JanusGraphSpeedBenchmark.basicCount	366.98275738303846 ms/op	481.22348171664004 ms/op	0.76
org.janusgraph.CQLMultiQueryBenchmark.getNeighborNames	42590.87824339 ms/op	63145.88842113334 ms/op	0.67
org.janusgraph.CQLMultiQueryBenchmark.getNames	42579.01455913667 ms/op	61622.81407393333 ms/op	0.69

It seems the GitHub CI environment varies a lot these days - thus, the benchmark result is not very reliable. I ran benchmarks locally and seems there's no regression or improvement by this PR.

FYI the benchmark result is: benchmark.md

@li-boxuan can rebase?

I'll merge this PR at the end of this week if no one else plans to review it.

This feature will be available for release 1.0.0? Because this is not available in Release candidate janusgraph-1.0.0-rc1.

Still getting the same error

gremlin> g.addV('user').property(T.id, 'f3695384-8442-447e-ba25-914e5b8deb08').property('name', 'Thirumal')
Cannot cast java.lang.String to java.lang.Number

@m-thirumal This feature is WIP. I have the code ready but need some time to clean it up and let the community review it. It may or may not be released in 1.0.0.