Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(23)

Unified Diff: include/private/SkOnce.h

Issue 1951013004: Simplify implementation of SkOnce to not need so many comments. (Closed) Base URL: https://skia.googlesource.com/skia.git@master
Patch Set: Created 4 years, 7 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: include/private/SkOnce.h
diff --git a/include/private/SkOnce.h b/include/private/SkOnce.h
index 507dcb5b8cf14dfdbecef9e9da81e7eea0e90a51..65334e3a26af1d63a26f247f11d1d7d9363ec5f4 100644
--- a/include/private/SkOnce.h
+++ b/include/private/SkOnce.h
@@ -29,44 +29,22 @@ public:
return;
}
- if (state == NotStarted) {
- // Try to claim the job of calling fn() by swapping from NotStarted to Calling.
- // See [1] below for why we use std::memory_order_acquire instead of relaxed.
- if (fState.compare_exchange_strong(state, Calling, std::memory_order_acquire)) {
- // Claimed! Call fn(), then mark this SkOnce as Done.
- fn(std::forward<Args>(args)...);
- return fState.store(Done, std::memory_order_release);
- }
+ // If it looks like no one has started calling fn(), try to claim that job.
+ if (state == NotStarted && fState.compare_exchange_strong(state, Claimed,
+ std::memory_order_relaxed)) {
+ // Great! We'll run fn() then notify the other threads by releasing Done into fState.
+ fn(std::forward<Args>(args)...);
+ return fState.store(Done, std::memory_order_release);
}
- while (state == Calling) {
- // Some other thread is calling fn(). Wait for them to finish.
- state = fState.load(std::memory_order_acquire);
- }
- SkASSERT(state == Done);
+ // Some other thread is calling fn().
+ // We'll just spin here acquiring until it releases Done into fState.
+ while (fState.load(std::memory_order_acquire) != Done) { /*spin*/ }
}
private:
- enum State : uint8_t { NotStarted, Calling, Done};
+ enum State : uint8_t { NotStarted, Claimed, Done};
std::atomic<uint8_t> fState{NotStarted};
};
-/* [1] Why do we compare_exchange_strong() with std::memory_order_acquire instead of relaxed?
- *
- * If we succeed, we really only need a relaxed compare_exchange_strong()... we're the ones
- * who are about to do a release store, so there's certainly nothing yet for an acquire to
- * synchronize with.
- *
- * If that compare_exchange_strong() fails, we're either in Calling or Done state.
- * Again, if we're in Calling state, relaxed would have been fine: the spin loop will
- * acquire up to the Calling thread's release store.
- *
- * But if that compare_exchange_strong() fails and we find ourselves in the Done state,
- * we've never done an acquire load to sync up to the store of that Done state.
- *
- * So on failure we need an acquire load. Generally the failure memory order cannot be
- * stronger than the success memory order, so we need acquire on success too. The single
- * memory order version of compare_exchange_strong() uses the same acquire order for both.
- */
-
#endif // SkOnce_DEFINED
« no previous file with comments | « no previous file | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698