yanglimingcn commented on code in PR #2358:
URL: https://github.com/apache/brpc/pull/2358#discussion_r1366530692


##########
src/bthread/bthread.cpp:
##########
@@ -111,38 +135,49 @@ static bool validate_bthread_min_concurrency(const char*, 
int32_t val) {
     BAIDU_SCOPED_LOCK(g_task_control_mutex);
     int concurrency = c->concurrency();
     if (val > concurrency) {
-        int added = c->add_workers(val - concurrency);
+        int added = bthread::add_workers_for_each_tag(val - concurrency);
         return added == (val - concurrency);
     } else {
         return true;
     }
 }
 
+static bool validate_bthread_tag_to_set(const char*, int32_t val) {
+    return val >= BTHREAD_TAG_INVALID && val < FLAGS_task_group_ntags;
+}
+
 __thread TaskGroup* tls_task_group_nosignal = NULL;
 
 BUTIL_FORCE_INLINE int
 start_from_non_worker(bthread_t* __restrict tid,
                       const bthread_attr_t* __restrict attr,
-                      void * (*fn)(void*),
+                      void* (*fn)(void*),
                       void* __restrict arg) {
     TaskControl* c = get_or_new_task_control();
     if (NULL == c) {
         return ENOMEM;
     }
+    TaskGroup* g = NULL;
     if (attr != NULL && (attr->flags & BTHREAD_NOSIGNAL)) {
         // Remember the TaskGroup to insert NOSIGNAL tasks for 2 reasons:
         // 1. NOSIGNAL is often for creating many bthreads in batch,
         //    inserting into the same TaskGroup maximizes the batch.
         // 2. bthread_flush() needs to know which TaskGroup to flush.
-        TaskGroup* g = tls_task_group_nosignal;
+        g = tls_task_group_nosignal;
         if (NULL == g) {
-            g = c->choose_one_group();
+            g = c->choose_one_group_with_tag(attr->tag);
             tls_task_group_nosignal = g;
         }
         return g->start_background<true>(tid, attr, fn, arg);
     }
-    return c->choose_one_group()->start_background<true>(
-        tid, attr, fn, arg);
+    g = c->choose_one_group_with_tag(attr ? attr->tag : BTHREAD_TAG_DEFAULT);
+    return g->start_background<true>(tid, attr, fn, arg);
+}
+
+// if tag is default or equal to thread local use thread local task group
+BUTIL_FORCE_INLINE bool can_run_thread_local(const bthread_attr_t* __restrict 
attr) {
+    return attr == nullptr || attr->tag == BTHREAD_TAG_DEFAULT ||

Review Comment:
   
这块感觉可以把BTHREAD_TAG_DEFAULT的校验去掉,因为现在brpc内部启动的bthread,要么是NULL,要么是默认的BTHREAD_ATTR_XXX,attr.tag都会被设置成BTHREAD_TAG_DEFAULT。
   
网络上来的请求启动的bthread都已经是设置好了tag的,用户启动的bthread,如果attr为null的话,会自然继承父bthread的属性。如果用户想要设置attr的话,tag也必须设置合理,可以使用bthread_self_tag()来获取当前tag上下文。



##########
src/bthread/bthread.cpp:
##########
@@ -111,38 +135,49 @@ static bool validate_bthread_min_concurrency(const char*, 
int32_t val) {
     BAIDU_SCOPED_LOCK(g_task_control_mutex);
     int concurrency = c->concurrency();
     if (val > concurrency) {
-        int added = c->add_workers(val - concurrency);
+        int added = bthread::add_workers_for_each_tag(val - concurrency);
         return added == (val - concurrency);
     } else {
         return true;
     }
 }
 
+static bool validate_bthread_tag_to_set(const char*, int32_t val) {
+    return val >= BTHREAD_TAG_INVALID && val < FLAGS_task_group_ntags;
+}
+
 __thread TaskGroup* tls_task_group_nosignal = NULL;
 
 BUTIL_FORCE_INLINE int
 start_from_non_worker(bthread_t* __restrict tid,
                       const bthread_attr_t* __restrict attr,
-                      void * (*fn)(void*),
+                      void* (*fn)(void*),
                       void* __restrict arg) {
     TaskControl* c = get_or_new_task_control();
     if (NULL == c) {
         return ENOMEM;
     }
+    TaskGroup* g = NULL;
     if (attr != NULL && (attr->flags & BTHREAD_NOSIGNAL)) {
         // Remember the TaskGroup to insert NOSIGNAL tasks for 2 reasons:
         // 1. NOSIGNAL is often for creating many bthreads in batch,
         //    inserting into the same TaskGroup maximizes the batch.
         // 2. bthread_flush() needs to know which TaskGroup to flush.
-        TaskGroup* g = tls_task_group_nosignal;
+        g = tls_task_group_nosignal;
         if (NULL == g) {
-            g = c->choose_one_group();
+            g = c->choose_one_group_with_tag(attr->tag);
             tls_task_group_nosignal = g;
         }
         return g->start_background<true>(tid, attr, fn, arg);
     }
-    return c->choose_one_group()->start_background<true>(
-        tid, attr, fn, arg);
+    g = c->choose_one_group_with_tag(attr ? attr->tag : BTHREAD_TAG_DEFAULT);
+    return g->start_background<true>(tid, attr, fn, arg);
+}
+
+// if tag is default or equal to thread local use thread local task group
+BUTIL_FORCE_INLINE bool can_run_thread_local(const bthread_attr_t* __restrict 
attr) {
+    return attr == nullptr || attr->tag == BTHREAD_TAG_DEFAULT ||

Review Comment:
   
这块感觉可以把BTHREAD_TAG_DEFAULT的校验去掉,因为现在brpc内部启动的bthread,要么是NULL,要么是默认的BTHREAD_ATTR_XXX,attr.tag都会被设置成BTHREAD_TAG_DEFAULT。
   
网络上来的请求启动的bthread都已经是设置好了tag的,用户启动的bthread,如果attr为null的话,会自然继承父bthread的属性。如果用户想要设置attr的话,tag也必须设置合理,可以使用bthread_self_tag()来获取当前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: dev-unsubscr...@brpc.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@brpc.apache.org
For additional commands, e-mail: dev-h...@brpc.apache.org

Reply via email to