Skip to content
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.

Sign up for GitHub

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

Jump to bottom

oltp traversal: optimize id2code for number id #1623

Merged
merged 3 commits into from
Oct 28, 2021
Merged

oltp traversal: optimize id2code for number id #1623

merged 3 commits into from
Oct 28, 2021

Conversation

javeme
Copy link
Contributor

@javeme javeme commented Oct 26, 2021

No description provided.

@javeme
optimize id2code for number id
66737c0 
Change-Id: I06a4139f89cb891a7323810bd4da43a3152069df
@javeme
refactor some method/var for oltp traversal
4eb601c 
Change-Id: I3976278f889890c4e1429457e4f129bed07b4559
@codecov
Copy link

codecov bot commented Oct 26, 2021 •
edited

Codecov Report

Merging #1623 (392fd41) into master (7512587) will decrease coverage by 0.08%.
The diff coverage is 59.16%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1623      +/-   ##
============================================
- Coverage     63.26%   63.18%   -0.09%     
- Complexity     6703     6711       +8     
============================================
  Files           418      418              
  Lines         34454    34497      +43     
  Branches       4764     4775      +11     
============================================
- Hits          21799    21797       -2     
- Misses        10407    10449      +42     
- Partials       2248     2251       +3
Impacted Files Coverage Δ
...versal/algorithm/records/record/Int2SetRecord.java 0.00% <0.00%> (ø)
...algorithm/strategy/ConcurrentTraverseStrategy.java 100.00% <ø> (ø)
...ph/util/collection/ConcurrentObjectIntMapping.java 0.00% <0.00%> (ø)
...m/baidu/hugegraph/util/collection/Int2IntsMap.java 75.00% <5.55%> (-25.00%) ⬇️
...h/traversal/algorithm/records/AbstractRecords.java 60.00% <30.00%> (-40.00%) ⬇️
.../algorithm/records/SingleWayMultiPathsRecords.java 53.96% <47.36%> (-0.72%) ⬇️
...versal/algorithm/records/record/Int2IntRecord.java 81.81% <50.00%> (-8.19%) ⬇️
...traversal/algorithm/records/record/RecordType.java 55.55% <50.00%> (-3.27%) ⬇️
.../util/collection/SingleThreadObjectIntMapping.java 81.48% <57.14%> (-8.00%) ⬇️
...rsal/algorithm/records/record/Int2ArrayRecord.java 90.00% <66.66%> (-10.00%) ⬇️
... and 18 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7512587...392fd41. Read the comment docs.

zhoney
zhoney requested changes Oct 27, 2021
View reviewed changes
...ph-core/src/main/java/com/baidu/hugegraph/traversal/algorithm/records/record/RecordType.java Outdated
// Java Collection Framework
ARRAY(1, "array"),
// One key with one int value
INT(2, "int"),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code is 1

javeme
javeme commented Oct 27, 2021
View reviewed changes
hugegraph-core/src/main/java/com/baidu/hugegraph/traversal/algorithm/ShortestPathTraverser.java Outdated
@@ -152,6 +151,11 @@ public Traverser(Id sourceV, Id targetV, Directions dir,
this.capacity = capacity;
}

public PathSet traverse(boolean all) {
return this.record.sourcesLessThanTargets() ?
this.forward(true) : this.backward(true);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pass param all

@javeme
tiny fix
392fd41 
Change-Id: I34bdec53b4e15e1de8a87a81484e96e50365efbd
@javeme javeme merged commit eef1f8b into master Oct 28, 2021
@javeme javeme deleted the id2code-oltp-traversal branch October 28, 2021 03:44
@LiJie20190102
Copy link

image
May I ask why this change was made? I recently encountered a problem where there is a large amount of data. The vertex ID is of type number, but I accidentally chose the String type when entering the graph. Therefore, when I execute olap, there will be an OOM issue; But when I switch to the Number type, no oom occurs. So I would like to ask why I am making code modifications here, and have I encountered similar problems as myself?

@LiJie20190102 LiJie20190102 mentioned this pull request Mar 1, 2024
Closed
1 task
@LiJie20190102
Copy link

image May I ask why this change was made? I recently encountered a problem where there is a large amount of data. The vertex ID is of type number, but I accidentally chose the String type when entering the graph. Therefore, when I execute olap, there will be an OOM issue; But when I switch to the Number type, no oom occurs. So I would like to ask why I am making code modifications here, and have I encountered similar problems as myself?

@javeme @zhoney @imbajin

@javeme
Copy link
Contributor Author

javeme commented Mar 3, 2024

@LiJie20190102 maybe the idMapping collection is too large, then cause OOM. you can try to check the large objects this way jmap -histo:live PID

@LiJie20190102
Copy link

LiJie20190102 commented Mar 4, 2024 •
edited

image

After my testing, I found that there are indeed more map objects created. That is to say, when I use the String type, I create more map objects; But when I switched to the Number type, there were fewer map objects.

I encountered the above issue while executing 'AllShortestPath' (org.apache.hugegraph.api.traverser.AllShortestPathsAPI#get),
params:
capacity:1000000
degree:10000
skipDegree:0
direction:BOTH
maxDepth:3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zhoney zhoney zhoney approved these changes

@imbajin imbajin imbajin approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@javeme @LiJie20190102 @zhoney @imbajin