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

Revert of Add SkRacy (https://codereview.chromium.org/371363004/)

Browse Source 
Reason for revert:
hidden symbol 'AnnotateBenignRaceSized' in obj/base/third_party/dynamic_annotations/libdynamic_annotations.a(obj/base/third_party/dynamic_annotations/dynamic_annotations.dynamic_annotations.o) is referenced by DSO lib/libblink_platform.so

Original issue's description:
> Add SkRacy
>
> 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:
>
> Committed: https://skia.googlesource.com/skia/+/d5e3e6ae1b3434ad1158f441902ff65f1eeaa3a7

R=reed@google.com, mtklein@chromium.org
TBR=mtklein@chromium.org, reed@google.com
NOTREECHECKS=true
NOTRY=true
BUG=skia:

Author: mtklein@google.com

Review URL: https://codereview.chromium.org/377693005
This commit is contained in:
mtklein 2014-07-08 14:06:46 -07:00 committed by Commit bot
parent 7b17547bc8
commit d3f3e5895e
6 changed files with 41 additions and 63 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

38
include/core/SkDynamicAnnotations.h
View File

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

16
include/core/SkPathRef.h
View File

@ -9,7 +9,6 @@
#ifndef SkPathRef_DEFINED
#define SkPathRef_DEFINED
#include "SkDynamicAnnotations.h"
#include "SkMatrix.h"
#include "SkPoint.h"
#include "SkRect.h"
@ -293,7 +292,7 @@ private:
SkDEBUGCODE(this->validate();)
SkASSERT(fBoundsIsDirty);
fIsFinite = ComputePtBounds(fBounds.get(), *this);
fIsFinite = ComputePtBounds(&fBounds, *this);
fBoundsIsDirty = false;
}
@ -301,7 +300,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 */
@ -433,12 +432,11 @@ private:
kMinSize = 256,
};
mutable SkTRacy<SkRect> fBounds;
mutable SkTRacy<uint8_t> fBoundsIsDirty;
mutable SkTRacy<SkBool8> fIsFinite; // only meaningful if bounds are valid
SkBool8 fIsOval;
uint8_t fSegmentMask;
mutable SkRect fBounds;
uint8_t fSegmentMask;
mutable uint8_t fBoundsIsDirty;
mutable SkBool8 fIsFinite; // only meaningful if bounds are valid
mutable SkBool8 fIsOval;
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,7 +9,6 @@
#define SkPixelRef_DEFINED
#include "SkBitmap.h"
#include "SkDynamicAnnotations.h"
#include "SkRefCnt.h"
#include "SkString.h"
#include "SkFlattenable.h"
@ -350,8 +349,8 @@ private:
LockRec fRec;
int fLockCount;
mutable SkTRacy<uint32_t> fGenerationID;
mutable SkTRacy<bool> fUniqueGenerationID;
mutable uint32_t fGenerationID;
mutable bool fUniqueGenerationID;
SkTDArray<GenIDChangeListener*> fGenIDChangeListeners; // pointers are owned

22
src/core/SkPathRef.cpp
View File

@ -30,7 +30,9 @@ SkPathRef::Editor::Editor(SkAutoTUnref<SkPathRef>* pathRef,
//////////////////////////////////////////////////////////////////////////////
SkPathRef* SkPathRef::CreateEmptyImpl() {
return SkNEW(SkPathRef);
SkPathRef* p = SkNEW(SkPathRef);
p->computeBounds(); // Preemptively avoid a race to clear fBoundsIsDirty.
return p;
}
SkPathRef* SkPathRef::CreateEmpty() {
@ -83,13 +85,13 @@ void SkPathRef::CreateTransformedCopy(SkAutoTUnref<SkPathRef>* dst,
if (canXformBounds) {
(*dst)->fBoundsIsDirty = false;
if (src.fIsFinite) {
matrix.mapRect((*dst)->fBounds.get(), src.fBounds);
if (!((*dst)->fIsFinite = (*dst)->fBounds->isFinite())) {
(*dst)->fBounds->setEmpty();
matrix.mapRect(&(*dst)->fBounds, 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;
@ -439,14 +441,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,3 +25,21 @@ 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?