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

Local resume task queue per task group. #2398

Open
wants to merge 21 commits into
base: master
Choose a base branch
from

Conversation

MrGuin
Copy link
Contributor

@MrGuin MrGuin commented Sep 27, 2023

What problem does this PR solve?

一个要解决的问题是 brpc worker 线程多的时候,由于大量的 signal_task 导致的性能雪崩。我们观察到 bthread_concurrency 多了后大量的 cpu 都会花费在 TaskContro::signal_task 以及 ParkingLot::signal 上,更具体是 ParkingLot::_pending_signal 这个原子变量上。signal_task 是一个热点函数,在每次发生 bthread 切换,新的 bthread 创建,以及等待中的 bthread 恢复执行时都会被调用;而 brpc 当前的 signal_task 的实现比较粗糙,每次 signal_task 都无差别地遍历对 parkinglot 做 signal,_pending_signal 的自增成为巨大的热点,会占用整体超过一半的 cpu。
这是我们在64核机器上 bthread_concurrency 设置为48时的 cpu profiling:
Screenshot 2023-10-16 at 12 14 53

就像代码注释里所说,

Current algorithm does not guarantee enough threads will be created to match caller's requests. But in another side, there's also many useless signalings according to current impl. Capping the concurrency is a good balance between performance and timeliness of scheduling.

针对 worker 唤醒,做的改动就是记录在 parkinglot 上等待的 worker num,只有有 worker 等待时才 signal_task;另外就是 steal_task 时让 worker 在 wait 在 parking lot 之前 busy poll 一小段时间,不那么频繁地 wait。

另外一个改动就是对于重新唤醒的 task 的处理。我们的场景 RPC bthread 会等待我们的 service 处理然后被唤醒,现有的 butex_wake 实现中虽然 brpc 的 worker 线程会立即切换到被唤醒的 bthread,但非 worker pthread 调用 butex_wake 只会把被唤醒的 bthread 给放进优先级很低的 _remote_rq。我们认为被唤醒的应该优先被处理,所以每个 TaskGroup 引入了额外的 _resume_rq 来保存由外部 pthread 唤醒的 bthread,在 wait_task 中优先检查。

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.(请遵循贡献者准则).

lzxddz and others added 14 commits June 14, 2023 14:08
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Add resume_rq for remote task and improve wait_task.
* add no_signal parameter to notify_one

* define guard for bthread_cond_signal
The latest code relies on:
 * C++11 -> C++17
 * Glog minimum version >= 0.6.0
@wwbmmm
Copy link
Contributor

wwbmmm commented Oct 12, 2023

Could you describe what functionality this PR has implemented and the general implementation approach?(可以描述一下这个PR实现了什么功能,以及大致的实现思路吗?)

@JimChengLin
Copy link
Contributor

JimChengLin commented Oct 14, 2023

我理解这个 MR 主要是两方面的工作:

  1. 给 ParkingLot 加了一个 waiter counter,如果没有 waiter 就不要走 futex wake 了,可以省 syscall。这是一个没有副作用的优化。还有个小建议是没有 waiter 的情况就不要计入 no singal 了,这时候所有 worker 已经火力全开了,攒着也没啥用。
2. 用 global queue 来替换 tg 的 remote task q。这个我考虑过一段时间了。我的 concern 在于这是 trade off 而不是纯优化。bthread 当前 steal 是随机 probe 所有的 tg。坏处是 tg 的 task 少的话,steal 会扫来扫去的。好处是 task 多的话,cache line 竞争会小。当然 global queue 也可以内部自己分多条 queue,但 trade off 是一样的。

就是这个steal策略有 2 个维度,一个是负载,一个是吞吐导向还是响应导向。如果负载低就应该用全局数据结构,反之要 shard。如果吞吐导向就应该 batch 唤醒和派发,反之应该激进唤醒 waiter。我不觉得有啥办法能把这 4 点就满足了。

华为最近 OSDI 2023 有篇 https://www.usenix.org/conference/osdi23/presentation/wang-jiawei
NSDI 2022 也有 https://www.[usenix.org/system/files/nsdi22-paper-mcclure_2.pdf](https://www.usenix.org/system/files/nsdi22-paper-mcclure_2.pdf)

tldr,我觉得线上作为默认调度都差点意思。BWoS 给 tokio 的 PR 好久还没合进去。

@MrGuin
Copy link
Contributor Author

MrGuin commented Oct 16, 2023

@wwbmmm 这个是我们对 brpc 做的一些改动,提给我们的 fork 的,不小心提到了这里,sorry。正好趁这个机会大家讨论一下,已补充上下文。

@wwbmmm
Copy link
Contributor

wwbmmm commented Oct 18, 2023

针对 worker 唤醒,做的改动就是记录在 parkinglot 上等待的 worker num,只有有 worker 等待时才 signal_task;另外就是 steal_task 时让 worker 在 wait 在 parking lot 之前 busy poll 一小段时间,不那么频繁地 wait。

这个是比较通用的一个优化,代码改动也比较小,可以单独提个PR吗?

另外一个改动就是对于重新唤醒的 task 的处理。我们的场景 RPC bthread 会等待我们的 service 处理然后被唤醒,现有的 butex_wake 实现中虽然 brpc 的 worker 线程会立即切换到被唤醒的 bthread,但非 worker pthread 调用 butex_wake 只会把被唤醒的 bthread 给放进优先级很低的 _remote_rq。我们认为被唤醒的应该优先被处理,所以每个 TaskGroup 引入了额外的 _resume_rq 来保存由外部 pthread 唤醒的 bthread,在 wait_task 中优先检查。

这个需求感觉有点定制化。有测过这个改之后的实际性能收益吗?

@MrGuin MrGuin force-pushed the resume_q_by_tg branch 2 times, most recently from de2ac82 to f9ce7cd Compare November 10, 2023 09:35
@chenBright
Copy link
Contributor

针对 worker 唤醒,做的改动就是记录在 parkinglot 上等待的 worker num,只有有 worker 等待时才 signal_task;另外就是 steal_task 时让 worker 在 wait 在 parking lot 之前 busy poll 一小段时间,不那么频繁地 wait。

@MrGuin 这个改动上线稳定运行了吗?可否推进一下合到社区。

@MrGuin
Copy link
Contributor Author

MrGuin commented Jun 20, 2024

针对 worker 唤醒,做的改动就是记录在 parkinglot 上等待的 worker num,只有有 worker 等待时才 signal_task;另外就是 steal_task 时让 worker 在 wait 在 parking lot 之前 busy poll 一小段时间,不那么频繁地 wait。

@MrGuin 这个改动上线稳定运行了吗?可否推进一下合到社区。

好的,我抽空针对这个提个 PR。我们内部一直在用,目前还算稳定。c7g.16xlarge 机型下四五十个 worker 线程可以达到两三百万的 qps,性能可以正常随着 worker 数 scale,不会雪崩了。

@zhengJade
Copy link
Contributor

这个有计划合并进来嘛,暂时没有的话,我就自己写了

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.

8 participants