MJY-HUST commented on code in PR #2645:
URL: https://github.com/apache/brpc/pull/2645#discussion_r1613198305
##########
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:
加读锁对性能没有太大的影响，因为通常情况下不会有线程持有写锁，加读锁不会阻塞线程。从火焰图中可以看出，瓶颈主要在调用futex_wait、futex_wake时产生，这部分是由于原有逻辑使用互斥锁时频繁阻塞线程造成的。
##########
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:
我觉得也有点多余，唯一一种场景是调用了return_keytable后，马上调用bthread_keytable_pool_destroy；此时bthread::tls_bls.keytable可能会被插入链表但是还没有被赋值为null。但是目前的链路中应该是不会发生这种现象的。
##########
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:
纯pthread调用bthread代码的情况下，这个pthread是不会创建tls list的。因为pool =
NULL，调不到butil::ThreadLocal<t>.get()方法
##########
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的话，造成的内存泄露反而会更多？
##########
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:
嗯，改过来了
--
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
