AuroraMiddleware/skia2
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Add SkRacy

Browse Source 
SkRacy<T> is a zero-overhead wrapper for a T, except it also
silences race warnings when TSAN is running.

Here we apply in several classes.  In SkMatrix and SkPathRef,
we use it to opportunistically cache some idempotent work.

In SkPixelRef, we wrap the genIDs.  We think the worst that
can happen here is we'll increment the global next-genID a
few times instead of once when we go to get another ID.

BUG=skia:
R=reed@google.com, mtklein@google.com

Author: mtklein@chromium.org

Review URL: https://codereview.chromium.org/371363004
This commit is contained in:
mtklein 2014-07-08 12:30:39 -07:00 committed by Commit bot
parent f9552230dc
commit d5e3e6ae1b
6 changed files with 63 additions and 41 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

38
include/core/SkDynamicAnnotations.h
View File

@ -19,6 +19,8 @@ extern "C" {
// TSAN provides these hooks.
void AnnotateIgnoreReadsBegin(const char* file, int line);
void AnnotateIgnoreReadsEnd(const char* file, int line);
void AnnotateBenignRaceSized(const char* file, int line,
const void* addr, long size, const char* desc);
} // extern "C"
// SK_ANNOTATE_UNPROTECTED_READ can wrap any variable read to tell TSAN to ignore that it appears to
@ -37,10 +39,46 @@ inline T SK_ANNOTATE_UNPROTECTED_READ(const volatile T& x) {
return read;
}
// Ignore racy reads and racy writes to this pointer, indefinitely.
// If at all possible, use the more precise SK_ANNOTATE_UNPROTECTED_READ.
template <typename T>
void SK_ANNOTATE_BENIGN_RACE(T* ptr) {
AnnotateBenignRaceSized(__FILE__, __LINE__, ptr, sizeof(*ptr), "SK_ANNOTATE_BENIGN_RACE");
}
#else // !DYNAMIC_ANNOTATIONS_ENABLED
#define SK_ANNOTATE_UNPROTECTED_READ(x) (x)
#define SK_ANNOTATE_BENIGN_RACE(ptr)
#endif
// Can be used to wrap values that are intentionally racy, usually small mutable cached values, e.g.
// - SkMatrix type mask
// - SkPathRef bounds
// - SkPixelRef genIDs
template <typename T>
class SkTRacy {
public:
SkTRacy() { SK_ANNOTATE_BENIGN_RACE(&fVal); }
operator const T&() const {
return fVal;
}
SkTRacy& operator=(const T& val) {
fVal = val;
return *this;
}
const T* get() const { return &fVal; }
T* get() { return &fVal; }
const T* operator->() const { return &fVal; }
T* operator->() { return &fVal; }
private:
T fVal;
};
#endif//SkDynamicAnnotations_DEFINED

5
include/core/SkMatrix.h
View File

@ -10,6 +10,7 @@
#ifndef SkMatrix_DEFINED
#define SkMatrix_DEFINED
#include "SkDynamicAnnotations.h"
#include "SkRect.h"
class SkString;
@ -643,7 +644,7 @@ private:
};
SkScalar fMat[9];
mutable uint32_t fTypeMask;
mutable SkTRacy<uint32_t> fTypeMask;
uint8_t computeTypeMask() const;
uint8_t computePerspectiveTypeMask() const;
@ -664,7 +665,7 @@ private:
void clearTypeMask(int mask) {
// only allow a valid mask
SkASSERT((mask & kAllMasks) == mask);
fTypeMask &= ~mask;
fTypeMask = fTypeMask & ~mask;
}
TypeMask getPerspectiveTypeMaskOnly() const {

16
include/core/SkPathRef.h
View File

@ -9,6 +9,7 @@
#ifndef SkPathRef_DEFINED
#define SkPathRef_DEFINED
#include "SkDynamicAnnotations.h"
#include "SkMatrix.h"
#include "SkPoint.h"
#include "SkRect.h"
@ -292,7 +293,7 @@ private:
SkDEBUGCODE(this->validate();)
SkASSERT(fBoundsIsDirty);
fIsFinite = ComputePtBounds(&fBounds, *this);
fIsFinite = ComputePtBounds(fBounds.get(), *this);
fBoundsIsDirty = false;
}
@ -300,7 +301,7 @@ private:
SkASSERT(rect.fLeft <= rect.fRight && rect.fTop <= rect.fBottom);
fBounds = rect;
fBoundsIsDirty = false;
fIsFinite = fBounds.isFinite();
fIsFinite = fBounds->isFinite();
}
/** Makes additional room but does not change the counts or change the genID */
@ -432,11 +433,12 @@ private:
kMinSize = 256,
};
mutable SkRect fBounds;
uint8_t fSegmentMask;
mutable uint8_t fBoundsIsDirty;
mutable SkBool8 fIsFinite; // only meaningful if bounds are valid
mutable SkBool8 fIsOval;
mutable SkTRacy<SkRect> fBounds;
mutable SkTRacy<uint8_t> fBoundsIsDirty;
mutable SkTRacy<SkBool8> fIsFinite; // only meaningful if bounds are valid
SkBool8 fIsOval;
uint8_t fSegmentMask;
SkPoint* fPoints; // points to begining of the allocation
uint8_t* fVerbs; // points just past the end of the allocation (verbs grow backwards)

5
include/core/SkPixelRef.h
View File

@ -9,6 +9,7 @@
#define SkPixelRef_DEFINED
#include "SkBitmap.h"
#include "SkDynamicAnnotations.h"
#include "SkRefCnt.h"
#include "SkString.h"
#include "SkFlattenable.h"
@ -349,8 +350,8 @@ private:
LockRec fRec;
int fLockCount;
mutable uint32_t fGenerationID;
mutable bool fUniqueGenerationID;
mutable SkTRacy<uint32_t> fGenerationID;
mutable SkTRacy<bool> fUniqueGenerationID;
SkTDArray<GenIDChangeListener*> fGenIDChangeListeners; // pointers are owned

22
src/core/SkPathRef.cpp
View File

@ -30,9 +30,7 @@ SkPathRef::Editor::Editor(SkAutoTUnref<SkPathRef>* pathRef,
//////////////////////////////////////////////////////////////////////////////
SkPathRef* SkPathRef::CreateEmptyImpl() {
SkPathRef* p = SkNEW(SkPathRef);
p->computeBounds(); // Preemptively avoid a race to clear fBoundsIsDirty.
return p;
return SkNEW(SkPathRef);
}
SkPathRef* SkPathRef::CreateEmpty() {
@ -85,13 +83,13 @@ void SkPathRef::CreateTransformedCopy(SkAutoTUnref<SkPathRef>* dst,
if (canXformBounds) {
(*dst)->fBoundsIsDirty = false;
if (src.fIsFinite) {
matrix.mapRect(&(*dst)->fBounds, src.fBounds);
if (!((*dst)->fIsFinite = (*dst)->fBounds.isFinite())) {
(*dst)->fBounds.setEmpty();
matrix.mapRect((*dst)->fBounds.get(), src.fBounds);
if (!((*dst)->fIsFinite = (*dst)->fBounds->isFinite())) {
(*dst)->fBounds->setEmpty();
}
} else {
(*dst)->fIsFinite = false;
(*dst)->fBounds.setEmpty();
(*dst)->fBounds->setEmpty();
}
} else {
(*dst)->fBoundsIsDirty = true;
@ -441,14 +439,14 @@ void SkPathRef::validate() const {
SkASSERT(this->currSize() ==
fFreeSpace + sizeof(SkPoint) * fPointCnt + sizeof(uint8_t) * fVerbCnt);
if (!fBoundsIsDirty && !fBounds.isEmpty()) {
if (!fBoundsIsDirty && !fBounds->isEmpty()) {
bool isFinite = true;
for (int i = 0; i < fPointCnt; ++i) {
SkASSERT(!fPoints[i].isFinite() || (
fBounds.fLeft - fPoints[i].fX < SK_ScalarNearlyZero &&
fPoints[i].fX - fBounds.fRight < SK_ScalarNearlyZero &&
fBounds.fTop - fPoints[i].fY < SK_ScalarNearlyZero &&
fPoints[i].fY - fBounds.fBottom < SK_ScalarNearlyZero));
fBounds->fLeft - fPoints[i].fX < SK_ScalarNearlyZero &&
fPoints[i].fX - fBounds->fRight < SK_ScalarNearlyZero &&
fBounds->fTop - fPoints[i].fY < SK_ScalarNearlyZero &&
fPoints[i].fY - fBounds->fBottom < SK_ScalarNearlyZero));
if (!fPoints[i].isFinite()) {
isFinite = false;
}

18
tools/tsan.supp
View File

@ -25,21 +25,3 @@ race:is_lcd_supported
race:RefFCI
race:SkString
race:SkPDF
# These race benignly as used by DMQuiltTask: skia:2725.
# Suppress while I look for a more focused way to silence this.
race:SkPixelRef::callGenIDChangeListeners
race:SkPixelRef::needsNewGenID
# SkPathRef caches its bounding box the first time it's needed.
# This will be fixed naturally once we create (from a single thread) a
# bounding-box hierarchy for SkRecord-based SkPictures; all bounds will come pre-cached.
# So just shut this up for now.
race:SkPathRef::computeBounds
# SkMatrix caches a type mask. If we race on this, we'll just calculate the same thing a few times.
race:SkMatrix::getType
race:SkMatrix::rectStaysRect
race:SkMatrix::getPerspectiveTypeMaskOnly
# TODO: some sort of SkRacy<T> to handle cases like SkMatrix, SkPathRef, SkPixelRef above?