diff options
| author |  mtklein <mtklein@chromium.org> | 2015-03-12 05:27:46 -0700 |
|---|---|---|
| committer |  Commit bot <commit-bot@chromium.org> | 2015-03-12 05:27:46 -0700 |
| commit | 172b45518a6a2c9c924cce124dcf27de88b03788 (patch) | |
| tree | a5ca81a816ea7f60082d3149ad0d155c14de5651 /src/core/SkTraceEvent.h | |
| parent | 97f43946fc2019d43048c16b22fee3096f08dfd8 (diff) | |
Clean up SkDynamicAnnotations.
Unprotected reads -> relaxed reads.
Unprotected write -> relaxed write.
The only unprotected write we had was in SkTraceEvent, which it looks like we nabbed from Chrome at some point and changed only to silence TSAN. Chrome's version uses AtomicWord / NoBarrier_Load / NoBarrier_Store, which boils down to the same as here, intptr_t / relaxed load / relaxed store.
This leaves one place where we're lying a bit to TSAN, in include/core/SkLazyPtr.h where we're doing a data-dependent consume load. We're telling TSAN it's consume, but telling any other compiler to compile it as relaxed, given how they all upgrade consume to acquire. This eliminates a barrier for us on ARM. How do you guys deal with this? Just use a consume memory order, take the hit, and hope compilers get smarter one day?
BUG=chromium:465721
No public API changes.
TBR=reed@google.com
Review URL: https://codereview.chromium.org/996763002
Diffstat (limited to 'src/core/SkTraceEvent.h')
| -rw-r--r-- | src/core/SkTraceEvent.h | 15 |
1 files changed, 4 insertions, 11 deletions
diff --git a/src/core/SkTraceEvent.h b/src/core/SkTraceEvent.h index 2c3c9b4181..5d7617b32b 100644 --- a/src/core/SkTraceEvent.h +++ b/src/core/SkTraceEvent.h @@ -161,8 +161,8 @@ #ifndef SkTraceEvent_DEFINED #define SkTraceEvent_DEFINED +#include "SkAtomics.h" #include "SkEventTracer.h" -#include "SkDynamicAnnotations.h" // By default, const char* argument values are assumed to have long-lived scope // and will not be copied. Use this macro to force a const char* to be copied. @@ -760,17 +760,10 @@ #define TRACE_EVENT_API_UPDATE_TRACE_EVENT_DURATION \ SkEventTracer::GetInstance()->updateTraceEventDuration -// These operations are atomic in the Chrome tracing implementation -// to cater to ARM's weak memory consistency; we're just doing read/ -// write here because it's not strictly needed for correctness. -// So says Nat. -// FIXME -// -// Skia addition: we mark these as unprotected reads and writes to shut up TSAN. - #define TRACE_EVENT_API_ATOMIC_WORD intptr_t -#define TRACE_EVENT_API_ATOMIC_LOAD(var) SK_ANNOTATE_UNPROTECTED_READ(var) -#define TRACE_EVENT_API_ATOMIC_STORE(var, value) SK_ANNOTATE_UNPROTECTED_WRITE(&var, value) +#define TRACE_EVENT_API_ATOMIC_LOAD(var) sk_atomic_load(&var, sk_memory_order_relaxed) +#define TRACE_EVENT_API_ATOMIC_STORE(var, value) \ + sk_atomic_store(&var, value, sk_memory_order_relaxed) // Defines visibility for classes in trace_event.h #define TRACE_EVENT_API_CLASS_EXPORT SK_API |