Skip to content

Commit

Permalink
absl: remove known_false condition in Mutex code
Browse files Browse the repository at this point in the history
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
  • Loading branch information
Abseil Team authored and copybara-github committed Sep 8, 2023
1 parent 38afe31 commit b9980dd
Showing 1 changed file with 3 additions and 8 deletions.
11 changes: 3 additions & 8 deletions absl/synchronization/mutex.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -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_ &&
Expand Down

0 comments on commit b9980dd

Please sign in to comment.