Skip to content

Commit

Permalink
Rollback:
Browse files Browse the repository at this point in the history
absl: remove special handling of Condition::kTrue
absl: remove known_false condition in Mutex code
There are some test breakages.

PiperOrigin-RevId: 563751370
Change-Id: Ie14dc799e0a0d286a7e1b47f0a9bbe59dfb23f70
  • Loading branch information
Abseil Team authored and copybara-github committed Sep 8, 2023
1 parent 6644e5b commit 792e55f
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 14 deletions.
34 changes: 25 additions & 9 deletions absl/synchronization/mutex.cc
Original file line number Diff line number Diff line change
Expand Up @@ -960,7 +960,8 @@ static PerThreadSynch* Enqueue(PerThreadSynch* head, SynchWaitParams* waitp,
} while (s->priority <= advance_to->priority);
// termination guaranteed because s->priority > head->priority
// and head is the end of a skip chain
} else if (waitp->how == kExclusive && waitp->cond == nullptr) {
} else if (waitp->how == kExclusive &&
Condition::GuaranteedEqual(waitp->cond, nullptr)) {
// An unlocker could be scanning the queue, but we know it will recheck
// the queue front for writers that have no condition, which is what s
// is, so an insert at front is safe.
Expand Down Expand Up @@ -1541,11 +1542,15 @@ bool Mutex::AwaitCommon(const Condition& cond, KernelTimeout t) {
SynchWaitParams waitp(how, &cond, t, nullptr /*no cvmu*/,
Synch_GetPerThreadAnnotated(this),
nullptr /*no cv_word*/);
int flags = kMuHasBlocked;
if (!Condition::GuaranteedEqual(&cond, nullptr)) {
flags |= kMuIsCond;
}
this->UnlockSlow(&waitp);
this->Block(waitp.thread);
ABSL_TSAN_MUTEX_POST_UNLOCK(this, TsanFlags(how));
ABSL_TSAN_MUTEX_PRE_LOCK(this, TsanFlags(how));
this->LockSlowLoop(&waitp, kMuHasBlocked | kMuIsCond);
this->LockSlowLoop(&waitp, flags);
bool res = waitp.cond != nullptr || // => cond known true from LockSlowLoop
EvalConditionAnnotated(&cond, this, true, false, how == kShared);
ABSL_TSAN_MUTEX_POST_LOCK(this, TsanFlags(how), 0);
Expand Down Expand Up @@ -1829,7 +1834,7 @@ bool Mutex::LockSlowWithDeadline(MuHow how, const Condition* cond,
SynchWaitParams waitp(how, cond, t, nullptr /*no cvmu*/,
Synch_GetPerThreadAnnotated(this),
nullptr /*no cv_word*/);
if (cond != nullptr) {
if (!Condition::GuaranteedEqual(cond, nullptr)) {
flags |= kMuIsCond;
}
if (unlock) {
Expand Down Expand Up @@ -2019,6 +2024,7 @@ 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 @@ -2122,7 +2128,7 @@ ABSL_ATTRIBUTE_NOINLINE void Mutex::UnlockSlow(SynchWaitParams* waitp) {
}
}
if (h->next->waitp->how == kExclusive &&
h->next->waitp->cond == nullptr) {
Condition::GuaranteedEqual(h->next->waitp->cond, nullptr)) {
// easy case: writer with no condition; no need to search
pw = h; // wake w, the successor of h (=pw)
w = h->next;
Expand Down Expand Up @@ -2205,8 +2211,10 @@ ABSL_ATTRIBUTE_NOINLINE void Mutex::UnlockSlow(SynchWaitParams* waitp) {
w_walk->wake = false;
if (w_walk->waitp->cond ==
nullptr || // no condition => vacuously true OR
// this thread's condition is true
EvalConditionIgnored(this, w_walk->waitp->cond)) {
(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))) {
if (w == nullptr) {
w_walk->wake = true; // can wake this waiter
w = w_walk;
Expand All @@ -2220,6 +2228,8 @@ 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 @@ -2712,11 +2722,17 @@ Condition::Condition(const bool* cond)
StoreCallback(dereference);
}

bool Condition::Eval() const { return (*this->eval_)(this); }
bool Condition::Eval() const {
// eval_ == null for kTrue
return (this->eval_ == nullptr) || (*this->eval_)(this);
}

bool Condition::GuaranteedEqual(const Condition* a, const Condition* b) {
if (a == nullptr || b == nullptr) {
return a == b;
// kTrue logic.
if (a == nullptr || a->eval_ == nullptr) {
return b == nullptr || b->eval_ == nullptr;
} else if (b == nullptr || b->eval_ == nullptr) {
return false;
}
// Check equality of the representative fields.
return a->eval_ == b->eval_ && a->arg_ == b->arg_ &&
Expand Down
8 changes: 3 additions & 5 deletions absl/synchronization/mutex.h
Original file line number Diff line number Diff line change
Expand Up @@ -833,10 +833,10 @@ class Condition {
#endif

// Function with which to evaluate callbacks and/or arguments.
bool (*const eval_)(const Condition*);
bool (*eval_)(const Condition*) = nullptr;

// Either an argument for a function call or an object for a method call.
void* const arg_;
void* arg_ = nullptr;

// Various functions eval_ can point to:
static bool CallVoidPtrFunction(const Condition*);
Expand All @@ -859,10 +859,8 @@ class Condition {
std::memcpy(callback, callback_, sizeof(*callback));
}

static bool AlwaysTrue(const Condition*) { return true; }

// Used only to create kTrue.
constexpr Condition() : eval_(AlwaysTrue), arg_(nullptr) {}
constexpr Condition() = default;
};

// -----------------------------------------------------------------------------
Expand Down

0 comments on commit 792e55f

Please sign in to comment.