summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGravatar Abseil Team <absl-team@google.com>2023-09-08 04:53:27 -0700
committerGravatar Copybara-Service <copybara-worker@google.com>2023-09-08 04:54:27 -0700
commitb9980dd454cf1849721d81b201173f9ced53f6bc (patch)
tree90ef505a1dbe55a4aa8d28c449debee7dd4ce71c
parent38afe317694fcc11fa9f4b27bd930023719cf699 (diff)
absl: remove known_false condition in Mutex code
Checking known_false condition only causes slow down because: 1. We already built skip list with equivalent conditions (and keep improving it on every Skip call). And when we built the skip list, we used more capable GuaranteedEqual function (it does not just check for equality of pointers, but for also for equality of function/arg). 2. Condition pointer are rarely equal even for equivalent conditions becuase temp Condition objects are usually created on the stack. We could call GuaranteedEqual(cond, known_false) instead of cond == known_false, but that slows down things even more (see point 1). So remove the known_false optimization. Benchmark results for this and the previous change: name old cpu/op new cpu/op delta BM_ConditionWaiters/0/1 36.0ns ± 0% 34.9ns ± 0% -3.02% (p=0.008 n=5+5) BM_ConditionWaiters/1/1 36.0ns ± 0% 34.9ns ± 0% -2.98% (p=0.008 n=5+5) BM_ConditionWaiters/2/1 35.9ns ± 0% 34.9ns ± 0% -3.03% (p=0.016 n=5+4) BM_ConditionWaiters/0/8 55.5ns ± 5% 49.8ns ± 3% -10.33% (p=0.008 n=5+5) BM_ConditionWaiters/1/8 36.2ns ± 0% 35.2ns ± 0% -2.90% (p=0.016 n=5+4) BM_ConditionWaiters/2/8 53.2ns ± 7% 48.3ns ± 7% ~ (p=0.056 n=5+5) BM_ConditionWaiters/0/64 295ns ± 1% 254ns ± 2% -13.73% (p=0.008 n=5+5) BM_ConditionWaiters/1/64 36.2ns ± 0% 35.2ns ± 0% -2.85% (p=0.008 n=5+5) BM_ConditionWaiters/2/64 290ns ± 6% 250ns ± 4% -13.68% (p=0.008 n=5+5) BM_ConditionWaiters/0/512 5.50µs ±12% 4.99µs ± 8% ~ (p=0.056 n=5+5) BM_ConditionWaiters/1/512 36.7ns ± 3% 35.2ns ± 0% -4.10% (p=0.008 n=5+5) BM_ConditionWaiters/2/512 4.44µs ±13% 4.01µs ± 3% -9.74% (p=0.008 n=5+5) BM_ConditionWaiters/0/4096 104µs ± 6% 101µs ± 3% ~ (p=0.548 n=5+5) BM_ConditionWaiters/1/4096 36.2ns ± 0% 35.1ns ± 0% -3.03% (p=0.008 n=5+5) BM_ConditionWaiters/2/4096 90.4µs ± 5% 85.3µs ± 7% ~ (p=0.222 n=5+5) BM_ConditionWaiters/0/8192 384µs ± 5% 367µs ± 7% ~ (p=0.222 n=5+5) BM_ConditionWaiters/1/8192 36.2ns ± 0% 35.2ns ± 0% -2.84% (p=0.008 n=5+5) BM_ConditionWaiters/2/8192 363µs ± 3% 316µs ± 7% -12.84% (p=0.008 n=5+5) PiperOrigin-RevId: 563717887 Change-Id: I9a62670628510d764a4f2f88a047abb8f85009e2
-rw-r--r--absl/synchronization/mutex.cc11
1 files changed, 3 insertions, 8 deletions
diff --git a/absl/synchronization/mutex.cc b/absl/synchronization/mutex.cc
index 9a25570c..dc34d8a9 100644
--- a/absl/synchronization/mutex.cc
+++ b/absl/synchronization/mutex.cc
@@ -2102,7 +2102,6 @@ ABSL_ATTRIBUTE_NOINLINE void Mutex::UnlockSlow(SynchWaitParams* waitp) {
// head of the list searched previously, or zero
PerThreadSynch* old_h = nullptr;
// a condition that's known to be false.
- const Condition* known_false = nullptr;
PerThreadSynch* wake_list = kPerThreadSynchNull; // list of threads to wake
intptr_t wr_wait = 0; // set to kMuWrWait if we wake a reader and a
// later writer could have acquired the lock
@@ -2289,10 +2288,8 @@ ABSL_ATTRIBUTE_NOINLINE void Mutex::UnlockSlow(SynchWaitParams* waitp) {
w_walk->wake = false;
if (w_walk->waitp->cond ==
nullptr || // no condition => vacuously true OR
- (w_walk->waitp->cond != known_false &&
- // this thread's condition is not known false, AND
- // is in fact true
- EvalConditionIgnored(this, w_walk->waitp->cond))) {
+ // this thread's condition is true
+ EvalConditionIgnored(this, w_walk->waitp->cond)) {
if (w == nullptr) {
w_walk->wake = true; // can wake this waiter
w = w_walk;
@@ -2306,8 +2303,6 @@ ABSL_ATTRIBUTE_NOINLINE void Mutex::UnlockSlow(SynchWaitParams* waitp) {
} else { // writer with true condition
wr_wait = kMuWrWait;
}
- } else { // can't wake; condition false
- known_false = w_walk->waitp->cond; // remember last false condition
}
if (w_walk->wake) { // we're waking reader w_walk
pw_walk = w_walk; // don't skip similar waiters
@@ -2814,7 +2809,7 @@ bool Condition::Eval() const { return (*this->eval_)(this); }
bool Condition::GuaranteedEqual(const Condition* a, const Condition* b) {
if (a == nullptr || b == nullptr) {
- return a == nullptr && b == nullptr;
+ return a == b;
}
// Check equality of the representative fields.
return a->eval_ == b->eval_ && a->arg_ == b->arg_ &&