chenBright commented on code in PR #3545:
URL: https://github.com/apache/brpc/pull/3545#discussion_r4089276723
##########
test/bthread_timer_thread_unittest.cpp:
##########
@@ -85,11 +101,11 @@ class TimeKeeper {
{
ASSERT_TRUE(!_run_times.empty());
long diff = timespec_diff_us(_run_times[0], expect_run_time);
- EXPECT_LE(labs(diff), 50000);
+ EXPECT_GE(diff, 0);
}
Review Comment:
Fixed in 53373a5d52bed4a1f06c00bcc108002c31181818 . expect_first_run() now
keeps the no-early-run check and also applies a deliberately generous 10-second
upper bound. This detects a stalled timer without restoring the former narrow
scheduling-latency assertion.
##########
test/bthread_futex_unittest.cpp:
##########
@@ -113,30 +116,55 @@ TEST(FutexTest, futex_wake_before_wait) {
}
void* dummy_waiter(void* lock) {
- bthread::futex_wait_private(lock, 0, nullptr);
+ timespec timeout = butil::seconds_to_timespec(10);
+ int rc;
+ do {
+ rc = bthread::futex_wait_private(lock, 0, &timeout);
+ } while (rc != 0 && errno == EINTR);
+ EXPECT_EQ(0, rc);
return nullptr;
}
TEST(FutexTest, futex_wake_many_waiters_perf) {
- int lock1 = 0;
- size_t N = 0;
- pthread_t th;
- for (; N < 1000 && !pthread_create(&th, nullptr, dummy_waiter, &lock1);
++N) {}
-
- sleep(1);
+ butil::atomic<int> lock1(0);
+ std::vector<pthread_t> threads;
+ for (size_t i = 0; i < 1000; ++i) {
+ pthread_t th;
+ if (pthread_create(&th, nullptr, dummy_waiter, &lock1) != 0) {
+ break;
+ }
+ threads.push_back(th);
+ }
+ ASSERT_FALSE(threads.empty());
+ size_t N = threads.size();
int nwakeup = 0;
+ int64_t wake_ns = 0;
+ int64_t deadline = butil::cpuwide_time_us() + 5000000L;
butil::Timer tm;
- tm.start();
- for (size_t i = 0; i < N; ++i) {
- nwakeup += bthread::futex_wake_private(&lock1, 1);
+ while (static_cast<size_t>(nwakeup) < N &&
+ butil::cpuwide_time_us() < deadline) {
+ tm.start();
+ int rc = bthread::futex_wake_private(&lock1, 1);
+ tm.stop();
+ EXPECT_GE(rc, 0);
+ if (rc > 0) {
+ nwakeup += rc;
+ wake_ns += tm.n_elapsed();
+ } else {
+ usleep(1000);
+ }
}
- tm.stop();
- printf("N=%lu, futex_wake a thread = %" PRId64 "ns\n", N, tm.n_elapsed() /
N);
- ASSERT_EQ(N, (size_t)nwakeup);
+ // Also release late waiters on failure; a wake alone is not persistent.
+ lock1.store(1);
+ bthread::futex_wake_private(&lock1, INT_MAX);
+ for (pthread_t th : threads) {
+ EXPECT_EQ(0, pthread_join(th, nullptr));
+ }
+ ASSERT_EQ(N, static_cast<size_t>(nwakeup));
+ printf("N=%lu, futex_wake a thread = %" PRId64 "ns\n", N, wake_ns / N);
Review Comment:
Fixed in 53373a5d52bed4a1f06c00bcc108002c31181818 . Each waiter now
publishes a registration acknowledgement before entering futex_wait_private.
The test waits for every created waiter before starting the wake measurement,
has a bounded wake loop, and retains cleanup/join handling if either
precondition times out.
--
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]
