apache / brpc

brpc is an Industrial-grade RPC framework using C++ Language, which is often used in high performance system such as Search, Storage, Machine learning, Advertisement, Recommendation etc. "brpc" means "better RPC".
https://brpc.apache.org
Apache License 2.0
16.05k stars 3.92k forks source link

Use butil::ThreadLocal to store keytable #2645

Closed MJY-HUST closed 21 hours ago

MJY-HUST commented 1 month ago

What problem does this PR solve?

Issue Number:

2635

Problem Summary: 当brpc server 下处理的应用执行时间较短(bthread生命周期短),且使用了bthread_local变量时,由于keytable 由bthread_keytable_pool_t中的一个全局链表维护,borrow_keytable、return_keytable时内部加互斥锁,导致锁成为瓶颈。 image

基于此,重新设计了bthread_keytable_pool_t: 新的bthread_keytable_pool_t结构体中,使用butil::ThreadLocal* list 保存TLS的KeyTable list。同时保留原有的free_keytables,不改变reserved_thread_local_data的语义,调用bthread_keytable_pool_reserve会在全局链表中预分配keytable.原有的互斥锁改为读写锁。

Changed:

Side effects:


Check List:

chenBright commented 1 month ago

object_pool应该没法完全解决 #1449 的问题吧,从object_pool取出来的KeyTable有可能已经有data了,先调用bthread_setspecific函数,旧data还是会泄漏。

chenBright commented 1 month ago

https://github.com/apache/brpc/blob/b601c89aec9042a1f509ddbdad478aa4e3f118fe/src/brpc/server.h#L170-L183 https://github.com/apache/brpc/blob/b601c89aec9042a1f509ddbdad478aa4e3f118fe/src/brpc/server.cpp#L888-L914

应该没法废弃bthread_keytable_pool_t ,因为Server已经用了bthread_keytable_pool_t了,并对外提供了与之相关的设置项。

改造bthread_keytable_pool_t,将free_keytables由全局改成TLS,这样mutex就可以废弃了。同时修改reserved_thread_local_data语义:每个TLS上预留的data数量。这样可行吗?

MJY-HUST commented 1 month ago

https://github.com/apache/brpc/blob/b601c89aec9042a1f509ddbdad478aa4e3f118fe/src/brpc/server.h#L170-L183

https://github.com/apache/brpc/blob/b601c89aec9042a1f509ddbdad478aa4e3f118fe/src/brpc/server.cpp#L888-L914

应该没法废弃bthread_keytable_pool_t ,因为Server已经用了bthread_keytable_pool_t了,并对外提供了与之相关的设置项。

改造bthread_keytable_pool_t,将free_keytables由全局改成TLS,这样mutex就可以废弃了。同时修改reserved_thread_local_data语义:每个TLS上预留的data数量。这样可行吗?

如果使用TLS的话,当server结束调用Join时,通过bthread_keytable_pool_destroy方法需要删除所有线程内保存的TLS free_keytables才行,感觉也不是很方便?

我的理解,目前的实现是一个server会绑定一个全局的bthread_keytable_pool_t,在server的声明周期内,server和其生成的bthread使用同一个pool,在server结束时会销毁所有创建的KeyTable,通过bthread_keytable_pool_t内的链表;然后会从全局的KeyInfo中删除所有注册的Key。但是使用全局一个链表会存在锁的竞争问题,不知道理解的对不对。

MJY-HUST commented 1 month ago

改造bthread_keytable_pool_t ,其中引入一个类似Object_pool的成员,通过在成员内部使用tls变量来避免加互斥锁。然后对bthread_keytable_pool_t 中的锁的使用从互斥锁改为读写锁,来维护destroyed的值。在return_keytable内使用读锁,bthread_keytable_pool_init,bthread_keytable_pool_destroy时加写锁。这样就可以避免在sever未退出时多个线程return_keytable产生的锁的竞争。 不知道这样是否可行

chenBright commented 1 month ago

我的理解,目前的实现是一个server会绑定一个全局的bthread_keytable_pool_t,在server的声明周期内,server和其生成的bthread使用同一个pool,在server结束时会销毁所有创建的KeyTable,通过bthread_keytable_pool_t内的链表;然后会从全局的KeyInfo中删除所有注册的Key。但是使用全局一个链表会存在锁的竞争问题,不知道理解的对不对。

嗯,我也是这样理解的。不过一般接口实现的瓶颈都是在业务逻辑,所以这个锁的影响应该不大。

chenBright commented 1 month ago

改造bthread_keytable_pool_t ,其中引入一个类似Object_pool的成员,通过在成员内部使用tls变量来避免加互斥锁。然后对bthread_keytable_pool_t 中的锁的使用从互斥锁改为读写锁,来维护destroyed的值。在return_keytable内使用读锁,bthread_keytable_pool_init,bthread_keytable_pool_destroy时加写锁。这样就可以避免在sever未退出时多个线程return_keytable产生的锁的竞争。 不知道这样是否可行

之前考虑过在bthread_keytable_pool_t中使用ObjectPool,但是ObjectPool好像不能实现reserved_thread_local_data吧?

destroyed应该可以用原子变量,就不用锁了吧。

chenBright commented 1 month ago

如果使用TLS的话,当server结束调用Join时,通过bthread_keytable_pool_destroy方法需要删除所有线程内保存的TLS free_keytables才行,感觉也不是很方便?

用butil::ThreadLocal应该挺方便的吧。 bthread_keytable_pool_init的时候,new一个butil::ThreadLocal。 bthread_keytable_pool_destroy的时候,delete。

MJY-HUST commented 1 month ago

如果使用TLS的话,当server结束调用Join时,通过bthread_keytable_pool_destroy方法需要删除所有线程内保存的TLS free_keytables才行,感觉也不是很方便?

用butil::ThreadLocal应该挺方便的吧。 bthread_keytable_pool_init的时候,new一个butil::ThreadLocal。 bthread_keytable_pool_destroy的时候,delete。

使用butil::ThreadLocal的话,new的object只有当线程结束才会释放,而目前基于bthread_keytable_pool_t的实现是在server Join后即清理释放所有的table,从KeyInfo中移除注册的key信息。如果使用butil::ThreadLocal,可以在delete后,析构所有的keytable吗?

MJY-HUST commented 1 month ago

改造bthread_keytable_pool_t ,其中引入一个类似Object_pool的成员,通过在成员内部使用tls变量来避免加互斥锁。然后对bthread_keytable_pool_t 中的锁的使用从互斥锁改为读写锁,来维护destroyed的值。在return_keytable内使用读锁,bthread_keytable_pool_init,bthread_keytable_pool_destroy时加写锁。这样就可以避免在sever未退出时多个线程return_keytable产生的锁的竞争。 不知道这样是否可行

之前考虑过在bthread_keytable_pool_t中使用ObjectPool,但是ObjectPool好像不能实现reserved_thread_local_data吧?

destroyed应该可以用原子变量,就不用锁了吧。

  1. 应该也不能直接使用Object_pool,因为它是一个单例,不能做到sever间的隔离。
  2. 我看ResourcePool获取对象的模式是如下图,那么reserve时初始化多个全局的block是否可行。 image
  3. return_keytable时,加读锁的临界区是判断destroyed + 将对象返回给pool,感觉不能直接使用原子变量
MJY-HUST commented 1 month ago

我的理解,目前的实现是一个server会绑定一个全局的bthread_keytable_pool_t,在server的声明周期内,server和其生成的bthread使用同一个pool,在server结束时会销毁所有创建的KeyTable,通过bthread_keytable_pool_t内的链表;然后会从全局的KeyInfo中删除所有注册的Key。但是使用全局一个链表会存在锁的竞争问题,不知道理解的对不对。

嗯,我也是这样理解的。不过一般接口实现的瓶颈都是在业务逻辑,所以这个锁的影响应该不大。

我们这边的场景是使用brpc server下面接了个rocksdb,当数据全被cache住时,service端的处理时间可能很短,导致瓶颈体现在了锁的竞争上 image

chenBright commented 1 month ago

如果使用TLS的话,当server结束调用Join时,通过bthread_keytable_pool_destroy方法需要删除所有线程内保存的TLS free_keytables才行,感觉也不是很方便?

用butil::ThreadLocal应该挺方便的吧。 bthread_keytable_pool_init的时候,new一个butil::ThreadLocal。 bthread_keytable_pool_destroy的时候,delete。

使用butil::ThreadLocal的话,new的object只有当线程结束才会释放,而目前基于bthread_keytable_pool_t的实现是在server Join后即清理释放所有的table,从KeyInfo中移除注册的key信息。如果使用butil::ThreadLocal,可以在delete后,析构所有的keytable吗?

butil::ThreadLocal析构时,会delete使用过butil::ThreadLocal的每个线程上的local data。应该可以满足需求吧。

chenBright commented 1 month ago

我看ResourcePool获取对象的模式是如下图,那么reserve时初始化多个全局的block是否可行。

ResourcePool也是单例哦。

return_keytable时,加读锁的临界区是判断destroyed + 将对象返回给pool,感觉不能直接使用原子变量

嗯嗯,return_keytable和bthread_keytable_pool_destroy需要互斥,不能用原子变量。

无论是ObjectPool还是ResourcePool的实现方式,即使destroyed=1,也是要将KeyTable返回给ObjectPool或者ResourcePool的吧。

MJY-HUST commented 1 month ago

如果使用TLS的话,当server结束调用Join时,通过bthread_keytable_pool_destroy方法需要删除所有线程内保存的TLS free_keytables才行,感觉也不是很方便?

用butil::ThreadLocal应该挺方便的吧。 bthread_keytable_pool_init的时候,new一个butil::ThreadLocal。 bthread_keytable_pool_destroy的时候,delete。

使用butil::ThreadLocal的话,new的object只有当线程结束才会释放,而目前基于bthread_keytable_pool_t的实现是在server Join后即清理释放所有的table,从KeyInfo中移除注册的key信息。如果使用butil::ThreadLocal,可以在delete后,析构所有的keytable吗?

butil::ThreadLocal析构时,会delete使用过butil::ThreadLocal的每个线程上的local data。应该可以满足需求吧。

请问一下这块逻辑具体在哪里呢?我没有看到butil::ThreadLocal这个类的定义。是ThreadLocalPointer吗? 当在一个线程上调用delete时,它是如何做到会delete使用过butil::ThreadLocal的每个线程上的local data,我没有看到类似链表或者list来存放所有线程的tls指针

chenBright commented 1 month ago

https://github.com/apache/brpc/blob/b601c89aec9042a1f509ddbdad478aa4e3f118fe/src/butil/thread_key.h#L144-L154

MJY-HUST commented 1 month ago

@chenBright hello,我根据讨论修改了实现,可以帮忙看一下么

chenBright commented 1 month ago

@wwbmmm 也看看这个PR

wwbmmm commented 4 weeks ago

对这个场景有点疑问,如果bthread生命周期很短,为什么还要使用bthread_local变量呢,用pthread local、局部变量或者通过函数参数传递不行吗

MJY-HUST commented 3 weeks ago

对这个场景有点疑问,如果bthread生命周期很短,为什么还要使用bthread_local变量呢,用pthread local、局部变量或者通过函数参数传递不行吗

场景是:使用brpc框架通信,server下面接着一个数据库。使用bthread pool用于处理数据库的查询请求,数据库内部的实现使用了bthread级别的锁和tls来避免阻塞线程。数据库内部需要使用一些thread_local变量(bthread_local)来保证其一致性并减少重复创建。 并不是所有的条件下bthread的生命周期都会很短,但是在一些场景下,如果数据都在缓存中,那么一个bthread的生命周期就会很短,此时单链表结构的keytable pool管理模式就会出现瓶颈。

wwbmmm commented 3 weeks ago

LGTM

Huixxi commented 1 day ago

请问下按照这样做了设计之后,有具体的性能测试报告么?能够直观的显示这种改造在特定场景下带来的性能提升

MJY-HUST commented 23 hours ago

请问下按照这样做了设计之后,有具体的性能测试报告么?能够直观的显示这种改造在特定场景下带来的性能提升

修改前: image image image

修改后: image image image

MJY-HUST commented 23 hours ago

请问下按照这样做了设计之后,有具体的性能测试报告么?能够直观的显示这种改造在特定场景下带来的性能提升

相同的负载下,修改前单链表维护的keytable,锁成为瓶颈。cpu被打满,sy时间占比高; 修改后,解除了锁的瓶颈,cpu利用率降低,QPS提升了一倍。火焰图上和加锁/解锁操作对应的futex_wait\futex_wake耗时部分消失