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

Unified Diff: include/private/SkOnce.h

Issue 1904483003: SkOnce: 2 bytes -> 1 byte (Closed) Base URL: https://skia.googlesource.com/skia.git@master
Patch Set: basic uint8_t type Created 4 years, 8 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 | include/private/SkOncePtr.h » ('j') | 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 d83b63f2837382f0179a0c987a67b8a2f979f3b7..1c68fb7da1396771aa3bed37365b321111270dde 100644
--- a/include/private/SkOnce.h
+++ b/include/private/SkOnce.h
@@ -8,9 +8,9 @@
#ifndef SkOnce_DEFINED
#define SkOnce_DEFINED
-#include "../private/SkSpinlock.h"
#include <atomic>
#include <utility>
+#include "SkTypes.h"
// SkOnce provides call-once guarantees for Skia, much like std::once_flag/std::call_once().
//
@@ -21,20 +21,50 @@ class SkOnce {
public:
template <typename Fn, typename... Args>
void operator()(Fn&& fn, Args&&... args) {
- // Vanilla double-checked locking.
- if (!fDone.load(std::memory_order_acquire)) {
- fLock.acquire();
- if (!fDone.load(std::memory_order_relaxed)) {
+ auto state = fState.load(std::memory_order_acquire);
+
+ if (state == Done) {
+ 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)...);
- fDone.store(true, std::memory_order_release);
+ return fState.store(Done, std::memory_order_release);
}
- fLock.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);
}
private:
- std::atomic<bool> fDone{false};
- SkSpinlock fLock;
+ enum State : uint8_t { NotStarted, Calling, 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 | include/private/SkOncePtr.h » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698