Skip to content

Bugfix: bthread_worker_usage could exceed bthread_worker_count#3009

Merged
wwbmmm merged 4 commits into
apache:masterfrom
chenBright:fix_bthread_worker_usage
Jul 23, 2025
Merged

Bugfix: bthread_worker_usage could exceed bthread_worker_count#3009
wwbmmm merged 4 commits into
apache:masterfrom
chenBright:fix_bthread_worker_usage

Conversation

@chenBright

@chenBright chenBright commented Jun 28, 2025

Copy link
Copy Markdown
Contributor

What problem does this PR solve?

Issue Number: resolve #1231 , resolve #2640 , resolve #2972

Problem Summary:

What is changed and the side effects?

Changed:

bthread_worker_usage should consist of _cumulated_cputime_ns and the CPU time of the task currently run by the worker.

Side effects:

  • Performance effects:

  • Breaking backward compatibility:


Check List:

@chenBright chenBright requested a review from Copilot June 28, 2025 02:32

This comment was marked as outdated.

@chenBright chenBright force-pushed the fix_bthread_worker_usage branch 3 times, most recently from 65e1b6e to 11497b0 Compare June 28, 2025 05:51
Comment thread src/bthread/task_control.cpp Outdated
@chenBright chenBright force-pushed the fix_bthread_worker_usage branch from 6ceff12 to eba4d84 Compare June 29, 2025 17:02
@chenBright chenBright requested a review from Copilot June 29, 2025 17:03

This comment was marked as outdated.

@chenBright chenBright force-pushed the fix_bthread_worker_usage branch from eba4d84 to 6aa0286 Compare June 29, 2025 17:36
@chenBright

chenBright commented Jun 30, 2025

Copy link
Copy Markdown
Contributor Author

@wwbmmm @yanglimingcn Please take a look.

@chenBright chenBright changed the title Bugfix: bthread_worker_usage would be greater than bthread_worker_count Bugfix: bthread_worker_usage could exceed bthread_worker_count Jul 3, 2025
@chenBright chenBright requested review from wwbmmm and yanglimingcn July 7, 2025 14:38
@chenBright

Copy link
Copy Markdown
Contributor Author

@wwbmmm @yanglimingcn Friendly ping!

Comment thread src/bthread/task_control.cpp Outdated
Comment thread src/bthread/task_group.h Outdated
Comment thread src/bthread/task_group.cpp Outdated
@chenBright

chenBright commented Jul 18, 2025

Copy link
Copy Markdown
Contributor Author
void* test(void*) {
    while (!brpc::IsAskedToQuit()) {
        sleep(2);
        bthread_yield();
    }
}

for (int i = 0; i < 3; ++i) {
    bthread_t tid;
    bthread_start_background(&tid, NULL, test, NULL);
}
image

Start 3 bthreads and loop sleep for 2s. The value of bthread_worker_usage is correct, and the extra 1 comes from the epoll bthread.

@chenBright chenBright force-pushed the fix_bthread_worker_usage branch 2 times, most recently from b7bfdb3 to 7d4ce67 Compare July 18, 2025 15:45
@chenBright chenBright force-pushed the fix_bthread_worker_usage branch from 7d4ce67 to bd1683b Compare July 18, 2025 15:50
@chenBright chenBright requested a review from Copilot July 19, 2025 06:35

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes a bug where bthread_worker_usage could exceed bthread_worker_count by refactoring how CPU time statistics are tracked for bthread workers. The fix ensures that worker usage calculations properly account for both cumulated CPU time and the current running task's elapsed time.

  • Introduces atomic 128-bit operations for thread-safe CPU time tracking
  • Refactors CPU time statistics to include both scheduling time and task type information
  • Consolidates prime offset generation into a shared utility function

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
src/bthread/task_group.h Adds AtomicInteger128 and CPUTimeStat classes, refactors TaskGroup member variables with default initialization
src/bthread/task_group.cpp Implements atomic operations and updates CPU time tracking logic in scheduling functions
src/bthread/task_control.h Renames method from get_cumulated_worker_time_with_tag to get_cumulated_worker_time
src/bthread/task_control.cpp Updates method calls and simplifies cumulated time calculation logic
src/bthread/prime_offset.h New utility header providing centralized prime offset generation functions
Load balancer files Updates to use the new centralized prime_offset() function instead of GenRandomStride()
src/brpc/load_balancer.h Removes deprecated GenRandomStride() function
Comments suppressed due to low confidence (1)

src/bthread/task_group.h:91

  • Misspelled word: 'Supress' should be 'Suppress'.
                                bthread_t* __restrict tid,

Comment thread src/bthread/task_group.cpp Outdated
Comment thread src/bthread/task_group.cpp Outdated
Comment thread src/bthread/task_group.h Outdated
Comment thread src/bthread/task_group.cpp Outdated
@chenBright chenBright force-pushed the fix_bthread_worker_usage branch from bd1683b to 9ca6a4b Compare July 19, 2025 07:19
Comment thread src/bthread/task_control.cpp Outdated
@wwbmmm

wwbmmm commented Jul 21, 2025

Copy link
Copy Markdown
Contributor

LGTM

@wwbmmm wwbmmm merged commit 05ec537 into apache:master Jul 23, 2025
15 checks passed
@chenBright chenBright deleted the fix_bthread_worker_usage branch July 23, 2025 02:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

5 participants