Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

modify bthread attribute with tag #2476

Merged
merged 1 commit into from
Jan 9, 2024

Conversation

yanglimingcn
Copy link
Contributor

What problem does this PR solve?

Issue Number:

Problem Summary:

What is changed and the side effects?

Changed:

Side effects:

  • Performance effects(性能影响):

  • Breaking backward compatibility(向后兼容性):


Check List:

  • Please make sure your changes are compilable(请确保你的更改可以通过编译).
  • When providing us with a new feature, it is best to add related tests(如果你向我们增加一个新的功能, 请添加相关测试).
  • Please follow Contributor Covenant Code of Conduct.(请遵循贡献者准则).

@yanglimingcn
Copy link
Contributor Author

@wwbmmm 我觉得brpc内部启动的协程,有些地方需要把bthread_tag设置正确,否则会存在跨线程池执行的问题,特别是socket.cpp这块。
其它地方我把握的不是很准确,请仔细review一下。

@yanglimingcn yanglimingcn force-pushed the feature/brpc_worker_tag branch 3 times, most recently from 4d7114a to e170408 Compare December 21, 2023 07:52
@wwbmmm
Copy link
Contributor

wwbmmm commented Dec 25, 2023

@wwbmmm 我觉得brpc内部启动的协程,有些地方需要把bthread_tag设置正确,否则会存在跨线程池执行的问题,特别是socket.cpp这块。 其它地方我把握的不是很准确,请仔细review一下。

类似BTHREAD_ATTR_NORMAL这些,能不能默认都设置成在本线程池执行?比如把tag的初始值设置成一个特殊值,表示在本线程池执行。因为BTHREAD_ATTR_NORMAL这些不只是在brpc框架会用,业务代码也会用到

@yanglimingcn yanglimingcn force-pushed the feature/brpc_worker_tag branch from e170408 to 55d7ef2 Compare December 25, 2023 06:30
@yanglimingcn
Copy link
Contributor Author

yanglimingcn commented Dec 25, 2023

@wwbmmm 我觉得brpc内部启动的协程,有些地方需要把bthread_tag设置正确,否则会存在跨线程池执行的问题,特别是socket.cpp这块。 其它地方我把握的不是很准确,请仔细review一下。

类似BTHREAD_ATTR_NORMAL这些,能不能默认都设置成在本线程池执行?比如把tag的初始值设置成一个特殊值,表示在本线程池执行。因为BTHREAD_ATTR_NORMAL这些不只是在brpc框架会用,业务代码也会用到

我这边加了一个BTHREAD_TAG_INVALID,是BTHREAD_ATTR_NORMAL这些的默认值,如果用户启动的协程用了这些的话,会在默认的local线程池中执行。

这么改之后,brpc内部的有些地方感觉也不需要改了

@yanglimingcn yanglimingcn force-pushed the feature/brpc_worker_tag branch 6 times, most recently from 430bdb2 to b4d2f99 Compare December 26, 2023 02:01
@yanglimingcn yanglimingcn force-pushed the feature/brpc_worker_tag branch from b4d2f99 to 9db7f2c Compare December 26, 2023 02:05
@yanglimingcn yanglimingcn requested a review from wwbmmm December 26, 2023 02:50
@wwbmmm
Copy link
Contributor

wwbmmm commented Dec 26, 2023

LGTM

@yanglimingcn
Copy link
Contributor Author

yanglimingcn commented Dec 30, 2023

LGTM

拆分线程池后,有些原来服务于一个线程池的routine现在服务于多个线程池,如果这个routine内部有bthread锁,就会发生问题,因为线程池之间不能被唤醒。另外就是跨线程池创建协程,使用了bthread锁。
brpc内部的routine有这种case吗?我看多数用的butil:Mutex,那内部routine使用锁的原则是怎么样的?
目前我能搜到的使用bthread::Mutex(bthread_mutex_*)的地方有,stream.cpp,id.cpp,有没有风险。

@wwbmmm
Copy link
Contributor

wwbmmm commented Jan 9, 2024

LGTM

拆分线程池后,有些原来服务于一个线程池的routine现在服务于多个线程池,如果这个routine内部有bthread锁,就会发生问题,因为线程池之间不能被唤醒。另外就是跨线程池创建协程,使用了bthread锁。 brpc内部的routine有这种case吗?我看多数用的butil:Mutex,那内部routine使用锁的原则是怎么样的? 目前我能搜到的使用bthread::Mutex(bthread_mutex_*)的地方有,stream.cpp,id.cpp,有没有风险。

线程池之间,能否支持唤醒呢?就像之前 pthread和bthread 也可以相互唤醒,可能需要调整下butex的相关逻辑

@wwbmmm wwbmmm merged commit ab5a496 into apache:master Jan 9, 2024
18 checks passed
@yanglimingcn
Copy link
Contributor Author

yanglimingcn commented Jan 9, 2024

LGTM

拆分线程池后,有些原来服务于一个线程池的routine现在服务于多个线程池,如果这个routine内部有bthread锁,就会发生问题,因为线程池之间不能被唤醒。另外就是跨线程池创建协程,使用了bthread锁。 brpc内部的routine有这种case吗?我看多数用的butil:Mutex,那内部routine使用锁的原则是怎么样的? 目前我能搜到的使用bthread::Mutex(bthread_mutex_*)的地方有,stream.cpp,id.cpp,有没有风险。

线程池之间,能否支持唤醒呢?就像之前 pthread和bthread 也可以相互唤醒,可能需要调整下butex的相关逻辑

我感觉线程池之间不需要这么做了,如果线程池之间有共享的内容,就退化到pthread_mutex就可以了。
另外那个FLAGS_usercode_in_pthread这个感觉可以做成每个线程池可以单独配置的。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants