feature: support partition concurrent compute #177

corgiboygsj · 2022-01-04T10:58:14Z

implement #167

codecov · 2022-01-04T11:08:57Z

Codecov Report

Merging #177 (5f4397f) into master (50b6147) will decrease coverage by 0.56%.
The diff coverage is 74.75%.

❗ Current head 5f4397f differs from pull request most recent head 89e92bc. Consider uploading reports for the commit 89e92bc to get more accurate results

@@             Coverage Diff              @@
##             master     #177      +/-   ##
============================================
- Coverage     87.32%   86.75%   -0.57%     
- Complexity     3151     3155       +4     
============================================
  Files           332      334       +2     
  Lines         11806    11897      +91     
  Branches       1053     1053              
============================================
+ Hits          10309    10321      +12     
- Misses          992     1069      +77     
- Partials        505      507       +2

Impacted Files	Coverage Δ
...graph/computer/core/network/DataClientManager.java	`85.36% <ø> (ø)`
.../baidu/hugegraph/computer/core/util/Consumers.java	`50.00% <50.00%> (ø)`
...graph/computer/core/sender/MessageSendManager.java	`78.40% <91.30%> (+1.47%)`	⬆️
...ugegraph/computer/core/compute/ComputeManager.java	`86.51% <92.00%> (-0.33%)`	⬇️
...raph/computer/core/compute/FileGraphPartition.java	`87.55% <94.11%> (+0.57%)`	⬆️
...ugegraph/computer/core/config/ComputerOptions.java	`100.00% <100.00%> (ø)`
...egraph/computer/core/input/WorkerInputManager.java	`100.00% <100.00%> (ø)`
...graph/computer/core/sender/MessageSendBuffers.java	`94.73% <100.00%> (+0.98%)`	⬆️
...aph/computer/core/sender/MessageSendPartition.java	`100.00% <100.00%> (ø)`
...aph/computer/core/sort/sorter/JavaInputSorter.java	`100.00% <100.00%> (ø)`
... and 19 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 50b6147...89e92bc. Read the comment docs.

javeme · 2022-01-11T07:06:49Z

computer-core/src/main/java/com/baidu/hugegraph/computer/core/sender/MessageSendManager.java

+        int partitionId = this.partitioner.partitionId(vertex.id());
+        WriteBuffers buffer = this.buffers.get(partitionId);
+
+        synchronized (buffer) {


Is it possible to use a more fine-grained lock?

I don't have any good ideas. Both writingBuffer and sortingBuffer may operated by this method, so I think WriteBuffers is the minimum granularity of lock.

computer-core/src/main/java/com/baidu/hugegraph/computer/core/sender/MessageSendManager.java

computer-core/src/main/java/com/baidu/hugegraph/computer/core/sort/sorter/JavaInputSorter.java

javeme · 2022-01-11T07:10:28Z

computer-core/src/main/java/com/baidu/hugegraph/computer/core/sort/sorter/JavaInputSorter.java

+
+    private static List<KvEntry> threadSortList() {
+        List<KvEntry> list = SORT_LOCAL.get();
+        list.clear();


it may cause all KvEntry released

corgiboygsj · 2022-01-14T08:52:45Z

@javeme MessageRecvManager#xxxPartitions can also merge files in parallel. Is it a good idea to pass the thread-pool to MessageRecvManager#xxxPartitions by ComputeManager?

javeme · 2022-01-14T12:15:27Z

@corgiboygsj Do you mean let receive-tasks and computing-tasks share the same thread pool?
Unified thread pool is beneficial in terms of management, but they may affect performance and stability with each other. I think you can have a try

javeme · 2022-01-14T12:21:45Z

computer-core/src/main/java/com/baidu/hugegraph/computer/core/util/Consumers.java

+
+public final class Consumers<V> {
+
+    private static final int CPU_CORE_NUM =


use a const instead to avoid container trap: aa80c09#diff-20996cff4e37b7ee95339fe118177993577be083e6034208aa0bb49ff29dd0ecR40

javeme · 2022-02-09T10:17:53Z

computer-core/src/main/java/com/baidu/hugegraph/computer/core/sender/MessageSendPartition.java

+import com.baidu.hugegraph.computer.core.common.ComputerContext;
+import com.baidu.hugegraph.computer.core.receiver.MessageStat;
+
+public class MessageSendPartition {


prefer rename to ThreadWriteBuffers

javeme · 2022-02-09T10:19:43Z

computer-core/src/main/java/com/baidu/hugegraph/computer/core/sender/MessageSendPartition.java

+        this.buffers = new ConcurrentHashMap<>();
+    }
+
+    public WriteBuffers get() {


rename to buffersForCurrentThread()?

javeme · 2022-02-09T10:24:58Z

computer-core/src/main/java/com/baidu/hugegraph/computer/core/sender/MessageSendPartition.java

+    public void resetMessageWritten() {
+        for (WriteBuffers buffer : this.buffers.values()) {
+            buffer.resetMessageWritten();
+        }
+    }
+
+    public MessageStat messageWritten() {
+        MessageStat partitionStat = new MessageStat();
+        for (WriteBuffers buffer : this.buffers.values()) {
+            partitionStat.increase(buffer.messageWritten());
+        }
+        return partitionStat;
+    }


may access by multi threads?

github-actions · 2022-03-11T21:11:01Z

Due to the lack of activity, the current pr is marked as stale and will be closed after 180 days, any update will remove the stale label

computer-core/src/main/java/com/baidu/hugegraph/computer/core/sort/sorter/JavaInputSorter.java

computer-core/src/main/java/com/baidu/hugegraph/computer/core/compute/ComputeManager.java

javeme · 2022-04-15T10:29:47Z

computer-core/src/main/java/com/baidu/hugegraph/computer/core/compute/ComputeManager.java


    private final ComputerContext context;
    private final Managers managers;

-    private final Map<Integer, FileGraphPartition<M>> partitions;
-    private final Computation<M> computation;
+    private final Map<Integer, FileGraphPartition> partitions;


is it right: multi threads may handle one partition, one MessageSendPartition hold multi buffers, one buffer each thread

javeme · 2022-04-15T10:35:50Z

computer-core/src/main/java/com/baidu/hugegraph/computer/core/sender/MessageSendManager.java

-            if (!buffer.isEmpty()) {
-                buffer.prepareSorting();
-                futures.add(this.sortThenSend(partitionId, type, buffer));
+            for (WriteBuffers buffer : partition.buffers()) {


can we ensure no thread access partition.buffers any more?

This method will only be called by main thread

github-actions · 2022-04-15T11:01:47Z

CLA Assistant Lite bot:
Thank you for your submission, we really appreciate it. Like many open-source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution. You can sign the CLA by just posting a Pull Request Comment same as the below format.

I have read the CLA Document and I hereby sign the CLA

_{You can retrigger this bot by commenting recheck in this Pull Request}

computer-core/src/main/java/com/baidu/hugegraph/computer/core/sort/sorter/JavaInputSorter.java

computer-test/src/main/java/com/baidu/hugegraph/computer/core/compute/input/EdgesInputTest.java

computer-core/src/main/java/com/baidu/hugegraph/computer/core/sender/MessageSendPartition.java

javeme · 2022-04-15T12:32:34Z

computer-core/src/main/java/com/baidu/hugegraph/computer/core/sender/MessageSendPartition.java

+    public void clear() {
+        this.buffers.clear();
+    }
+
+    public void resetMessageWritten() {
+        for (WriteBuffers buffer : this.buffers.values()) {
+            buffer.resetMessageWritten();
+        }
+    }
+
+    public MessageStat messageWritten() {


add synchronized for the 3 methods?

can do this, but they are only accessed by the main thread

computer-core/src/main/java/com/baidu/hugegraph/computer/core/sort/sorter/JavaInputSorter.java

javeme · 2022-04-29T04:37:04Z

ci error:
beyondstorage/setup-hdfs@master is not allowed to be used in apache/incubator-hugegraph-computer.

codecov · 2022-05-19T02:26:14Z

Codecov Report

Merging #177 (bca36db) into master (0d2e6df) will decrease coverage by 0.63%.
The diff coverage is 74.75%.

@@             Coverage Diff              @@
##             master     #177      +/-   ##
============================================
- Coverage     87.37%   86.73%   -0.64%     
- Complexity     3153     3167      +14     
============================================
  Files           332      334       +2     
  Lines         11806    11962     +156     
  Branches       1053     1068      +15     
============================================
+ Hits          10315    10375      +60     
- Misses          988     1073      +85     
- Partials        503      514      +11

Impacted Files	Coverage Δ
...graph/computer/core/network/DataClientManager.java	`85.36% <ø> (ø)`
.../baidu/hugegraph/computer/core/util/Consumers.java	`50.00% <50.00%> (ø)`
...graph/computer/core/sender/MessageSendManager.java	`78.40% <91.30%> (+1.47%)`	⬆️
...ugegraph/computer/core/compute/ComputeManager.java	`86.51% <92.00%> (-0.33%)`	⬇️
...raph/computer/core/compute/FileGraphPartition.java	`87.55% <94.11%> (+0.57%)`	⬆️
...ugegraph/computer/core/config/ComputerOptions.java	`100.00% <100.00%> (ø)`
...egraph/computer/core/input/WorkerInputManager.java	`100.00% <100.00%> (ø)`
...graph/computer/core/sender/MessageSendBuffers.java	`94.73% <100.00%> (+0.98%)`	⬆️
...aph/computer/core/sender/MessageSendPartition.java	`100.00% <100.00%> (ø)`
...aph/computer/core/sort/sorter/JavaInputSorter.java	`100.00% <100.00%> (ø)`
... and 14 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 0d2e6df...bca36db. Read the comment docs.

corgiboygsj marked this pull request as draft January 4, 2022 10:58

corgiboygsj force-pushed the concurrent-compute branch from 9c5719d to a16e64b Compare January 4, 2022 10:58

corgiboygsj force-pushed the concurrent-compute branch 7 times, most recently from 2250d87 to db82aa5 Compare January 11, 2022 04:48

javeme reviewed Jan 11, 2022

View reviewed changes

javeme reviewed Jan 14, 2022

View reviewed changes

coderzc previously approved these changes Jan 18, 2022

View reviewed changes

corgiboygsj dismissed coderzc’s stale review via 1085cee January 18, 2022 06:39

corgiboygsj force-pushed the concurrent-compute branch from db82aa5 to 1085cee Compare January 18, 2022 06:39

corgiboygsj force-pushed the concurrent-compute branch from 5f4397f to 8cc49b6 Compare February 9, 2022 09:44

javeme reviewed Feb 9, 2022

View reviewed changes

github-actions bot added the inactive label Mar 11, 2022

javeme reviewed Apr 15, 2022

View reviewed changes

corgiboygsj force-pushed the concurrent-compute branch from 212bd8f to 89e92bc Compare April 15, 2022 11:04

javeme reviewed Apr 15, 2022

View reviewed changes

corgiboygsj marked this pull request as ready for review April 25, 2022 09:07

corgiboygsj force-pushed the concurrent-compute branch 2 times, most recently from 2956516 to 09b0e51 Compare April 25, 2022 09:15

javeme previously approved these changes Apr 25, 2022

View reviewed changes

github-actions bot removed the inactive label Apr 29, 2022

corgiboygsj added 8 commits May 19, 2022 10:16

support partition parallel compute

6c01c62

synchronized WriteBuffers operate

3bb7437

improve code

26a959d

rebuild parallel use Consumers

16e0a67

improve code

92bff4a

improve buffer sort

28fb6eb

each partition use multi buffer to avoid use synchronized

8f5437f

improve code

bca36db

corgiboygsj dismissed javeme’s stale review via bca36db May 19, 2022 02:16

corgiboygsj force-pushed the concurrent-compute branch from 09b0e51 to bca36db Compare May 19, 2022 02:16

javeme approved these changes May 19, 2022

View reviewed changes

coderzc approved these changes May 19, 2022

View reviewed changes

coderzc merged commit bcaef85 into master May 19, 2022

coderzc deleted the concurrent-compute branch May 19, 2022 06:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feature: support partition concurrent compute #177

feature: support partition concurrent compute #177

corgiboygsj commented Jan 4, 2022 •

edited by coderzc

Loading

codecov bot commented Jan 4, 2022 •

edited

Loading

javeme Jan 11, 2022

corgiboygsj Jan 14, 2022 •

edited

Loading

javeme Jan 11, 2022

corgiboygsj commented Jan 14, 2022

javeme commented Jan 14, 2022

javeme Jan 14, 2022

javeme Feb 9, 2022

javeme Feb 9, 2022

javeme Feb 9, 2022

github-actions bot commented Mar 11, 2022

javeme Apr 15, 2022

corgiboygsj Apr 15, 2022

javeme Apr 15, 2022

corgiboygsj Apr 15, 2022

github-actions bot commented Apr 15, 2022 •

edited

Loading

javeme Apr 15, 2022

corgiboygsj Apr 25, 2022

javeme commented Apr 29, 2022

codecov bot commented May 19, 2022


		public final class Consumers<V> {

		private static final int CPU_CORE_NUM =

feature: support partition concurrent compute #177

feature: support partition concurrent compute #177

Conversation

corgiboygsj commented Jan 4, 2022 • edited by coderzc Loading

codecov bot commented Jan 4, 2022 • edited Loading

Codecov Report

Choose a reason for hiding this comment

corgiboygsj Jan 14, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

corgiboygsj commented Jan 14, 2022

javeme commented Jan 14, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Mar 11, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Apr 15, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

javeme commented Apr 29, 2022

codecov bot commented May 19, 2022

Codecov Report

corgiboygsj commented Jan 4, 2022 •

edited by coderzc

Loading

codecov bot commented Jan 4, 2022 •

edited

Loading

corgiboygsj Jan 14, 2022 •

edited

Loading

github-actions bot commented Apr 15, 2022 •

edited

Loading