chenBright commented on code in PR #2645:
URL: https://github.com/apache/brpc/pull/2645#discussion_r1611151985
##########
src/bthread/key.cpp:
##########
@@ -226,14 +267,15 @@ void return_keytable(bthread_keytable_pool_t* pool,
KeyTable* kt) {
delete kt;
return;
}
- std::unique_lock<pthread_mutex_t> mu(pool->mutex);
+ pthread_rwlock_rdlock(&pool->rwlock);
if (pool->destroyed) {
- mu.unlock();
+ pthread_rwlock_unlock(&pool->rwlock);
delete kt;
return;
}
- kt->next = (KeyTable*)pool->free_keytables;
- pool->free_keytables = kt;
+ kt->next = pool->list->get()->keytable;
+ pool->list->get()->keytable = kt;
+ pthread_rwlock_unlock(&pool->rwlock);
Review Comment:
1. 同上，操作TLS list不需要在锁内吧？
2. 需要限制tls list缓存上限吗？
##########
src/bthread/key.cpp:
##########
@@ -226,14 +267,15 @@ void return_keytable(bthread_keytable_pool_t* pool,
KeyTable* kt) {
delete kt;
return;
}
- std::unique_lock<pthread_mutex_t> mu(pool->mutex);
+ pthread_rwlock_rdlock(&pool->rwlock);
if (pool->destroyed) {
- mu.unlock();
+ pthread_rwlock_unlock(&pool->rwlock);
delete kt;
return;
}
- kt->next = (KeyTable*)pool->free_keytables;
- pool->free_keytables = kt;
+ kt->next = pool->list->get()->keytable;
+ pool->list->get()->keytable = kt;
+ pthread_rwlock_unlock(&pool->rwlock);
}
static void cleanup_pthread(void* arg) {
Review Comment:
cleanup_pthread应该还需要调pool->list->reset(NULL)，否则pool->list析构的时候就double delete。
##########
src/bthread/types.h:
##########
@@ -83,8 +83,17 @@ inline std::ostream& operator<<(std::ostream& os,
bthread_key_t key) {
}
#endif // __cplusplus
+namespace bthread{
+class KeyTableList;
+}
+
+namespace butil {
+template <typename T> class ThreadLocal;
+}
+
typedef struct {
- pthread_mutex_t mutex;
+ pthread_rwlock_t rwlock;
+ butil::ThreadLocal<bthread::KeyTableList>* list;
void* free_keytables;
Review Comment:
butil::ThreadLocalbthread::KeyTableList* -> void* ?
##########
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:
1. `&& (pool->list->get()->keytable || pool->free_keytables)`这个条件好像没有必要？
2. 操作TLS list不需要加读锁吧？
##########
src/bthread/key.cpp:
##########
@@ -330,11 +358,12 @@ int
bthread_keytable_pool_getstat(bthread_keytable_pool_t* pool,
LOG(ERROR) << "Param[pool] or Param[stat] is NULL";
return EINVAL;
}
- std::unique_lock<pthread_mutex_t> mu(pool->mutex);
+ pthread_rwlock_wrlock(&pool->rwlock);
size_t count = 0;
bthread::KeyTable* p = (bthread::KeyTable*)pool->free_keytables;
for (; p; p = p->next, ++count) {}
stat->nfree = count;
+ pthread_rwlock_unlock(&pool->rwlock);
Review Comment:
好像读锁就可以了？
--
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
