You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by GitBox <gi...@apache.org> on 2022/04/24 09:09:14 UTC

[GitHub] [incubator-doris] mrhhsg opened a new pull request, #9193: [Feature]Add multi-thread probe to hash join

mrhhsg opened a new pull request, #9193:
URL: https://github.com/apache/incubator-doris/pull/9193

   # Proposed changes
   
   Issue Number: close #xxx
   
   ## Problem Summary:
   
   Describe the overview of changes.
   
   ## Checklist(Required)
   
   1. Does it affect the original behavior: (Yes/No/I Don't know)
   2. Has unit tests been added: (Yes/No/No Need)
   3. Has document been added or modified: (Yes/No/No Need)
   4. Does it need to update dependencies: (Yes/No)
   5. Are there any changes that cannot be rolled back: (Yes/No)
   
   ## Further comments
   
   If this is a relatively large or complex change, kick off the discussion at [dev@doris.apache.org](mailto:dev@doris.apache.org) by explaining why you chose the solution you did and what alternatives you considered, etc...
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] yangzhg commented on a diff in pull request #9193: [Feature]Add multi-thread probe to hash join

Posted by GitBox <gi...@apache.org>.
yangzhg commented on code in PR #9193:
URL: https://github.com/apache/incubator-doris/pull/9193#discussion_r858502751


##########
fe/fe-core/src/main/java/org/apache/doris/qe/SessionVariable.java:
##########
@@ -71,6 +71,7 @@ public class SessionVariable implements Serializable, Writable {
     public static final String SQL_SAFE_UPDATES = "sql_safe_updates";
     public static final String NET_BUFFER_LENGTH = "net_buffer_length";
     public static final String CODEGEN_LEVEL = "codegen_level";
+    public static final String HASH_JOIN_PROBE_THREAD_COUNT = "hash_join_probe_thread_count";

Review Comment:
   ```suggestion
       public static final String HASH_JOIN_PROBE_THREAD_NUM = "hash_join_probe_thread_num";
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] morningman commented on pull request #9193: [Feature]Add multi-thread probe to hash join

Posted by GitBox <gi...@apache.org>.
morningman commented on PR #9193:
URL: https://github.com/apache/incubator-doris/pull/9193#issuecomment-1107840664

   hi @mrhhsg , you should at least create an issue to describe what problem you solved, and how it impacts the performance.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] yangzhg commented on a diff in pull request #9193: [Feature]Add multi-thread probe to hash join

Posted by GitBox <gi...@apache.org>.
yangzhg commented on code in PR #9193:
URL: https://github.com/apache/incubator-doris/pull/9193#discussion_r858500882


##########
be/src/vec/exec/join/vhash_join_node.cpp:
##########
@@ -636,6 +653,58 @@ struct ProcessHashTableProbe {
     ProfileCounter* _probe_side_output_timer;
 };
 
+class ProbeThreadPool {
+public:
+    typedef std::function<void(int thread_id)> WorkFunction;
+    struct Task {
+    public:
+        WorkFunction work_function;
+    };
+
+    ProbeThreadPool(uint32_t num_threads, uint32_t queue_size)
+            : _work_queue(queue_size), _shutdown(false) {
+        for (int i = 0; i < num_threads; ++i) {
+            _threads.create_thread(
+                    std::bind<void>(std::mem_fn(&ProbeThreadPool::work_thread), this, i));
+        }
+    }
+
+    ~ProbeThreadPool() {
+        shutdown();
+        join();
+    }
+
+    bool offer(Task task) { return _work_queue.blocking_put(task); }
+
+    void shutdown() {
+        _shutdown = true;
+        _work_queue.shutdown();
+    }
+
+    virtual void join() { _threads.join_all(); }

Review Comment:
   why this method virtual



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] zbtzbtzbt commented on pull request #9193: [Feature]Add multi-thread probe to hash join

Posted by GitBox <gi...@apache.org>.
zbtzbtzbt commented on PR #9193:
URL: https://github.com/apache/incubator-doris/pull/9193#issuecomment-1108030233

   question: what is the number of `multi-thread`? 2 or 4 or 8?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] mrhhsg commented on a diff in pull request #9193: [Feature]Add multi-thread probe to hash join

Posted by GitBox <gi...@apache.org>.
mrhhsg commented on code in PR #9193:
URL: https://github.com/apache/incubator-doris/pull/9193#discussion_r858548631


##########
be/src/vec/exec/join/vhash_join_node.cpp:
##########
@@ -636,6 +653,58 @@ struct ProcessHashTableProbe {
     ProfileCounter* _probe_side_output_timer;
 };
 
+class ProbeThreadPool {
+public:
+    typedef std::function<void(int thread_id)> WorkFunction;
+    struct Task {
+    public:
+        WorkFunction work_function;
+    };
+
+    ProbeThreadPool(uint32_t num_threads, uint32_t queue_size)
+            : _work_queue(queue_size), _shutdown(false) {
+        for (int i = 0; i < num_threads; ++i) {
+            _threads.create_thread(
+                    std::bind<void>(std::mem_fn(&ProbeThreadPool::work_thread), this, i));
+        }
+    }
+
+    ~ProbeThreadPool() {
+        shutdown();
+        join();
+    }
+
+    bool offer(Task task) { return _work_queue.blocking_put(task); }
+
+    void shutdown() {
+        _shutdown = true;
+        _work_queue.shutdown();
+    }
+
+    virtual void join() { _threads.join_all(); }

Review Comment:
   Yes, I will remove it.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [doris] github-actions[bot] commented on pull request #9193: [Feature]Add multi-thread probe to hash join

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #9193:
URL: https://github.com/apache/doris/pull/9193#issuecomment-1289807166

   We're closing this PR because it hasn't been updated in a while.
   This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable.
   If you'd like to revive this PR, please reopen it and feel free a maintainer to remove the Stale tag!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [doris] github-actions[bot] closed pull request #9193: [Feature]Add multi-thread probe to hash join

Posted by GitBox <gi...@apache.org>.
github-actions[bot] closed pull request #9193: [Feature]Add multi-thread probe to hash join
URL: https://github.com/apache/doris/pull/9193


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] yiguolei commented on pull request #9193: [Feature]Add multi-thread probe to hash join

Posted by GitBox <gi...@apache.org>.
yiguolei commented on PR #9193:
URL: https://github.com/apache/incubator-doris/pull/9193#issuecomment-1110483464

   @Gabriel39 What do you think about this PR?  Will it be covered when we implement pipeline?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] mrhhsg commented on pull request #9193: [Feature]Add multi-thread probe to hash join

Posted by GitBox <gi...@apache.org>.
mrhhsg commented on PR #9193:
URL: https://github.com/apache/incubator-doris/pull/9193#issuecomment-1107990300

   > hi @mrhhsg , you should at least create an issue to describe what problem you solved, and how it impacts the performance.
   
   Hi @morningman , very sorry for missing the issue, and I have created one.
   And this PR can significantly improve the speed of hash join ( from 2s994ms to 1s637ms)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] mrhhsg commented on pull request #9193: [Feature]Add multi-thread probe to hash join

Posted by GitBox <gi...@apache.org>.
mrhhsg commented on PR #9193:
URL: https://github.com/apache/incubator-doris/pull/9193#issuecomment-1108043334

   > question: what is the number of `multi-thread`? 2 or 4 or 8?
   
   1 thread for child get_next, 4 threads for probe. 
   The number is configurable.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org