remove sk_atomic_compare_exchange
Whatever nonsense SkPicture::uniqueID() was doing, it doesn't
need to do it. It can just get its unique ID normally.
I've ported SkEventTracer in the straightforward way.
Change-Id: I103e7e05258ad49e1e3f333fc907f039cef3f8c9
Reviewed-on: https://skia-review.googlesource.com/c/174280
Commit-Queue: Mike Klein <mtklein@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: Mike Klein <mtklein@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
diff --git a/include/core/SkPicture.h b/include/core/SkPicture.h
index 7ab5392..fd055fe 100644
--- a/include/core/SkPicture.h
+++ b/include/core/SkPicture.h
@@ -148,7 +148,7 @@
@return identifier for SkPicture
*/
- uint32_t uniqueID() const;
+ uint32_t uniqueID() const { return fUniqueID; }
/** Returns storage containing SkData describing SkPicture, using optional custom
encoders.
@@ -278,7 +278,7 @@
struct SkPictInfo createHeader() const;
class SkPictureData* backport() const;
- mutable uint32_t fUniqueID;
+ uint32_t fUniqueID;
};
#endif
diff --git a/include/private/SkAtomics.h b/include/private/SkAtomics.h
index d062246..a5ac6c9 100644
--- a/include/private/SkAtomics.h
+++ b/include/private/SkAtomics.h
@@ -42,20 +42,6 @@
return std::atomic_fetch_add_explicit(ap, val, mo);
}
-template <typename T>
-bool sk_atomic_compare_exchange(T* ptr, T* expected, T desired,
- std::memory_order success = std::memory_order_seq_cst,
- std::memory_order failure = std::memory_order_seq_cst) {
- // All values of success are valid.
- SkASSERT(failure == std::memory_order_relaxed ||
- failure == std::memory_order_seq_cst ||
- failure == std::memory_order_acquire ||
- failure == std::memory_order_consume);
- SkASSERT(failure <= success);
- std::atomic<T>* ap = reinterpret_cast<std::atomic<T>*>(ptr);
- return std::atomic_compare_exchange_strong_explicit(ap, expected, desired, success, failure);
-}
-
// ~~~~~~~~ Very Legacy APIs ~~~~~~~~~
//
// Here are shims for our very old atomics API, to be weaned off of. They use
diff --git a/src/core/SkPicture.cpp b/src/core/SkPicture.cpp
index 603dea9..8e7a711 100644
--- a/src/core/SkPicture.cpp
+++ b/src/core/SkPicture.cpp
@@ -7,7 +7,6 @@
#include "SkPicture.h"
-#include "SkAtomics.h"
#include "SkImageGenerator.h"
#include "SkMathPriv.h"
#include "SkPictureCommon.h"
@@ -18,6 +17,7 @@
#include "SkPictureRecorder.h"
#include "SkSerialProcs.h"
#include "SkTo.h"
+#include <atomic>
// When we read/write the SkPictInfo via a stream, we have a sentinel byte right after the info.
// Note: in the read/write buffer versions, we have a slightly different convention:
@@ -33,22 +33,11 @@
/* SkPicture impl. This handles generic responsibilities like unique IDs and serialization. */
-SkPicture::SkPicture() : fUniqueID(0) {}
-
-uint32_t SkPicture::uniqueID() const {
- static uint32_t gNextID = 1;
- uint32_t id = sk_atomic_load(&fUniqueID, std::memory_order_relaxed);
- while (id == 0) {
- uint32_t next = sk_atomic_fetch_add(&gNextID, 1u);
- if (sk_atomic_compare_exchange(&fUniqueID, &id, next,
- std::memory_order_relaxed,
- std::memory_order_relaxed)) {
- id = next;
- } else {
- // sk_atomic_compare_exchange replaced id with the current value of fUniqueID.
- }
- }
- return id;
+SkPicture::SkPicture() {
+ static std::atomic<uint32_t> nextID{1};
+ do {
+ fUniqueID = nextID.fetch_add(+1, std::memory_order_relaxed);
+ } while (fUniqueID == 0);
}
static const char kMagic[] = { 's', 'k', 'i', 'a', 'p', 'i', 'c', 't' };
diff --git a/src/utils/SkEventTracer.cpp b/src/utils/SkEventTracer.cpp
index 6f8141b..7251c26 100644
--- a/src/utils/SkEventTracer.cpp
+++ b/src/utils/SkEventTracer.cpp
@@ -5,9 +5,9 @@
* found in the LICENSE file.
*/
-#include "SkAtomics.h"
#include "SkEventTracer.h"
#include "SkOnce.h"
+#include <atomic>
#include <stdlib.h>
@@ -40,21 +40,20 @@
};
// We prefer gUserTracer if it's been set, otherwise we fall back on a default tracer;
-static SkEventTracer* gUserTracer = nullptr;
+static std::atomic<SkEventTracer*> gUserTracer{nullptr};
bool SkEventTracer::SetInstance(SkEventTracer* tracer) {
SkEventTracer* expected = nullptr;
- if (!sk_atomic_compare_exchange(&gUserTracer, &expected, tracer)) {
+ if (!gUserTracer.compare_exchange_strong(expected, tracer)) {
delete tracer;
return false;
}
- // An atomic load during process shutdown is probably overkill, but safe overkill.
- atexit([]() { delete sk_atomic_load(&gUserTracer); });
+ atexit([]() { delete gUserTracer.load(); });
return true;
}
SkEventTracer* SkEventTracer::GetInstance() {
- if (SkEventTracer* tracer = sk_atomic_load(&gUserTracer, std::memory_order_acquire)) {
+ if (auto tracer = gUserTracer.load(std::memory_order_acquire)) {
return tracer;
}
static SkOnce once;