chenBright commented on code in PR #2645:
URL: https://github.com/apache/brpc/pull/2645#discussion_r1615404666


##########
src/bthread/key.cpp:
##########
@@ -234,11 +234,12 @@ class KeyTableList {
 };
 
 static KeyTable* borrow_keytable(bthread_keytable_pool_t* pool) {
-    if (pool != NULL && (pool->list->get()->keytable || pool->free_keytables)) 
{
+    butil::ThreadLocal<bthread::KeyTableList>* list = 
(butil::ThreadLocal<bthread::KeyTableList>*)pool->list;

Review Comment:
   这一行太长了,用auto或者换行?



##########
src/bthread/key.cpp:
##########
@@ -273,8 +274,9 @@ void return_keytable(bthread_keytable_pool_t* pool, 
KeyTable* kt) {
         delete kt;
         return;
     }
-    kt->next = pool->list->get()->keytable;
-    pool->list->get()->keytable = kt;
+    butil::ThreadLocal<bthread::KeyTableList>* list = 
(butil::ThreadLocal<bthread::KeyTableList>*)pool->list;

Review Comment:
   同上



##########
src/bthread/key.cpp:
##########
@@ -204,14 +205,54 @@ class BAIDU_CACHELINE_ALIGNMENT KeyTable {
     SubKeyTable* _subs[KEY_1STLEVEL_SIZE];
 };
 
+class KeyTableList {
+public:
+    KeyTableList() {
+        keytable = NULL;
+    }
+    ~KeyTableList() {
+        bthread::TaskGroup* const g = bthread::tls_task_group;
+        bthread::KeyTable* old_kt = bthread::tls_bls.keytable;
+        while (keytable) {
+            bthread::KeyTable* kt = keytable;
+            keytable = kt->next;
+            bthread::tls_bls.keytable = kt;
+            if (g) {
+                g->current_task()->local_storage.keytable = kt;
+            }

Review Comment:
   哦哦 框架内部可以做到这种用法



##########
src/bthread/key.cpp:
##########
@@ -204,14 +205,54 @@ class BAIDU_CACHELINE_ALIGNMENT KeyTable {
     SubKeyTable* _subs[KEY_1STLEVEL_SIZE];
 };
 
+class KeyTableList {
+public:
+    KeyTableList() {
+        keytable = NULL;
+    }
+    ~KeyTableList() {
+        bthread::TaskGroup* const g = bthread::tls_task_group;
+        bthread::KeyTable* old_kt = bthread::tls_bls.keytable;
+        while (keytable) {
+            bthread::KeyTable* kt = keytable;
+            keytable = kt->next;
+            bthread::tls_bls.keytable = kt;
+            if (g) {
+                g->current_task()->local_storage.keytable = kt;
+            }
+            delete kt;
+            if (old_kt == kt) {
+                old_kt = NULL;
+            }
+        }
+        bthread::tls_bls.keytable = old_kt;
+        if(g) {
+            g->current_task()->local_storage.keytable = old_kt;
+        }
+    }
+    KeyTable* keytable;
+};
+
 static KeyTable* borrow_keytable(bthread_keytable_pool_t* pool) {
-    if (pool != NULL && pool->free_keytables) {
-        BAIDU_SCOPED_LOCK(pool->mutex);
-        KeyTable* p = (KeyTable*)pool->free_keytables;
+    if (pool != NULL && (pool->list->get()->keytable || pool->free_keytables)) 
{
+        pthread_rwlock_rdlock(&pool->rwlock);
+        KeyTable* p = pool->list->get()->keytable;
         if (p) {
-            pool->free_keytables = p->next;
+            pool->list->get()->keytable = p->next;
+            pthread_rwlock_unlock(&pool->rwlock);
             return p;
         }
+        pthread_rwlock_unlock(&pool->rwlock);

Review Comment:
   > 如果不析构pool->list的话,造成的内存泄露反而会更多?
   
   ThreadLocal对象本身应该还好吧



-- 
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