Copilot commented on code in PR #3697:
URL: https://github.com/apache/brpc/pull/3697#discussion_r4435276560
##########
src/bthread/semaphore.cpp:
##########
@@ -110,16 +110,18 @@ static int bthread_sem_wait_impl(bthread_sem_t* sem,
const struct timespec* abst
static inline int bthread_sem_post(bthread_sem_t* sem, size_t num) {
if (num > 0) {
+ // Publish preceding writes to successful acquire-CAS waiters,
including
+ // those that consume permits without entering butex_wait().
((butil::atomic<unsigned>*)sem->butex)
- ->fetch_add(num, butil::memory_order_relaxed);
- const size_t sampling_range = nullptr != bthread::g_cp &&
sem->enable_csite ?
+ ->fetch_add(num, butil::memory_order_release);
+ size_t sampling_range = nullptr != bthread::g_cp && sem->enable_csite ?
bvar::is_collectable(&bthread::g_cp_sl) :
bvar::INVALID_SAMPLING_RANGE;
- const int64_t start_ns = bvar::is_sampling_range_valid(sampling_range)
?
- butil::cpuwide_time_ns() : -1;
+ int64_t start_ns = bvar::is_sampling_range_valid(sampling_range) ?
+ butil::cpuwide_time_ns() : -1;
bthread::butex_wake_n(sem->butex, num);
if (start_ns > 0) {
- const int64_t end_ns = butil::cpuwide_time_ns();
- const bthread_contention_site_t csite{end_ns - start_ns,
sampling_range};
+ int64_t end_ns = butil::cpuwide_time_ns();
+ bthread_contention_site_t csite{end_ns - start_ns, sampling_range};
Review Comment:
The release-ordering change is correct, but this hunk also removes `const`
from the locals `sampling_range`, `start_ns`, `end_ns`, and `csite` in
`bthread_sem_post()`. Nothing in this PR needs that, and the same locals are
`const` in `bthread_sem_wait_impl` (`src/bthread/semaphore.cpp:65-66` and
`:89-90`). Restore the original declarations so the diff only changes the
`fetch_add` ordering and its 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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
