replace SkNVRefCnt with SkRefCnt
SkNVRefCnt trades a small amount of code size (vtable) and runtime
(vptr) memory usage for a larger amount of code size (templating). It
was written back in a time when all we were really thinking about was
runtime memory usage, so I'm curious to see where performance, code
size, and memory usage all move if it's removed.
Looking at the types I've changed here, my guess is that performance and
memory usage will be basically unchanged, and that code size will drop a
bit. Nothing else it's nicer to have only one ref-counting base class.
Change-Id: I7d56a2b9e2b9fb000ff97792159ea1ff4f5e6f13
Reviewed-on: https://skia-review.googlesource.com/c/166203
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
diff --git a/include/core/SkColorSpace.h b/include/core/SkColorSpace.h
index 26771f3..5ca9b45 100644
--- a/include/core/SkColorSpace.h
+++ b/include/core/SkColorSpace.h
@@ -62,7 +62,7 @@
float fF;
};
-class SK_API SkColorSpace : public SkNVRefCnt<SkColorSpace> {
+class SK_API SkColorSpace : public SkRefCnt {
public:
/**
* Create the sRGB color space.
diff --git a/include/core/SkData.h b/include/core/SkData.h
index 931749a..c279d52 100644
--- a/include/core/SkData.h
+++ b/include/core/SkData.h
@@ -19,7 +19,7 @@
* but the actual ptr that is returned (by data() or bytes()) is guaranteed
* to always be the same for the life of this instance.
*/
-class SK_API SkData final : public SkNVRefCnt<SkData> {
+class SK_API SkData final : public SkRefCnt {
public:
/**
* Returns the number of bytes stored.
@@ -158,7 +158,7 @@
static sk_sp<SkData> MakeEmpty();
private:
- friend class SkNVRefCnt<SkData>;
+ friend class SkRefCnt;
ReleaseProc fReleaseProc;
void* fReleaseProcContext;
void* fPtr;
diff --git a/include/core/SkRefCnt.h b/include/core/SkRefCnt.h
index bc6ccd8..ee2710d 100644
--- a/include/core/SkRefCnt.h
+++ b/include/core/SkRefCnt.h
@@ -163,44 +163,6 @@
}
}
-///////////////////////////////////////////////////////////////////////////////
-
-// This is a variant of SkRefCnt that's Not Virtual, so weighs 4 bytes instead of 8 or 16.
-// There's only benefit to using this if the deriving class does not otherwise need a vtable.
-template <typename Derived>
-class SkNVRefCnt {
-public:
- SkNVRefCnt() : fRefCnt(1) {}
- ~SkNVRefCnt() { SkASSERTF(1 == getRefCnt(), "NVRefCnt was %d", getRefCnt()); }
-
- // Implementation is pretty much the same as SkRefCntBase. All required barriers are the same:
- // - unique() needs acquire when it returns true, and no barrier if it returns false;
- // - ref() doesn't need any barrier;
- // - unref() needs a release barrier, and an acquire if it's going to call delete.
-
- bool unique() const { return 1 == fRefCnt.load(std::memory_order_acquire); }
- void ref() const { (void)fRefCnt.fetch_add(+1, std::memory_order_relaxed); }
- void unref() const {
- if (1 == fRefCnt.fetch_add(-1, std::memory_order_acq_rel)) {
- // restore the 1 for our destructor's assert
- SkDEBUGCODE(fRefCnt.store(1, std::memory_order_relaxed));
- delete (const Derived*)this;
- }
- }
- void deref() const { this->unref(); }
-
-private:
- mutable std::atomic<int32_t> fRefCnt;
- int32_t getRefCnt() const {
- return fRefCnt.load(std::memory_order_relaxed);
- }
-
- SkNVRefCnt(SkNVRefCnt&&) = delete;
- SkNVRefCnt(const SkNVRefCnt&) = delete;
- SkNVRefCnt& operator=(SkNVRefCnt&&) = delete;
- SkNVRefCnt& operator=(const SkNVRefCnt&) = delete;
-};
-
///////////////////////////////////////////////////////////////////////////////////////////////////
/**
diff --git a/include/core/SkTextBlob.h b/include/core/SkTextBlob.h
index 907241c..81c44fc 100644
--- a/include/core/SkTextBlob.h
+++ b/include/core/SkTextBlob.h
@@ -36,7 +36,7 @@
run consists of glyphs, SkPaint, and position. Only parts of SkPaint related to
fonts and text rendering are used by run.
*/
-class SK_API SkTextBlob final : public SkNVRefCnt<SkTextBlob> {
+class SK_API SkTextBlob final : public SkRefCnt {
public:
/** Returns conservative bounding box. Uses SkPaint associated with each glyph to
@@ -138,7 +138,7 @@
const SkDeserialProcs& procs);
private:
- friend class SkNVRefCnt<SkTextBlob>;
+ friend class SkRefCnt;
class RunRecord;
enum GlyphPositioning : uint8_t;
diff --git a/include/core/SkVertices.h b/include/core/SkVertices.h
index 63a27a0..f36a877 100644
--- a/include/core/SkVertices.h
+++ b/include/core/SkVertices.h
@@ -17,7 +17,7 @@
/**
* An immutable set of vertex data that can be used with SkCanvas::drawVertices.
*/
-class SK_API SkVertices : public SkNVRefCnt<SkVertices> {
+class SK_API SkVertices : public SkRefCnt {
public:
// BoneIndices indicates which (of a maximum of 4 bones) a given vertex will interpolate
// between. To indicate that a slot is not used, the convention is to assign the bone index
@@ -250,7 +250,7 @@
SkVertices() {}
// these are needed since we've manually sized our allocation (see Builder::init)
- friend class SkNVRefCnt<SkVertices>;
+ friend class SkRefCnt;
void operator delete(void* p);
static sk_sp<SkVertices> Alloc(int vCount, int iCount, uint32_t builderFlags,
diff --git a/include/private/GrSkSLFPFactoryCache.h b/include/private/GrSkSLFPFactoryCache.h
index 40e001a..7440791 100644
--- a/include/private/GrSkSLFPFactoryCache.h
+++ b/include/private/GrSkSLFPFactoryCache.h
@@ -19,7 +19,7 @@
// For thread safety, it is important that GrSkSLFP only interact with the cache from methods that
// are only called from within the rendering thread, like onCreateGLSLInstance and
// onGetGLSLProcessorKey.
-class GrSkSLFPFactoryCache : public SkNVRefCnt<GrSkSLFPFactoryCache> {
+class GrSkSLFPFactoryCache : public SkRefCnt {
public:
// Returns a factory by its numeric index, or null if no such factory exists. Indices are
// allocated by GrSkSLFP::NewIndex().
diff --git a/include/private/SkPathRef.h b/include/private/SkPathRef.h
index d250fc3..630fdfb 100644
--- a/include/private/SkPathRef.h
+++ b/include/private/SkPathRef.h
@@ -39,7 +39,7 @@
* logical verb or the last verb in memory).
*/
-class SK_API SkPathRef final : public SkNVRefCnt<SkPathRef> {
+class SK_API SkPathRef final : public SkRefCnt {
public:
class Editor {
public:
diff --git a/modules/skottie/include/Skottie.h b/modules/skottie/include/Skottie.h
index 2f832bc..b0a2d24 100644
--- a/modules/skottie/include/Skottie.h
+++ b/modules/skottie/include/Skottie.h
@@ -125,7 +125,7 @@
virtual void onAnnotation(const char key[], const char value[]) = 0;
};
-class SK_API Animation : public SkNVRefCnt<Animation> {
+class SK_API Animation : public SkRefCnt {
public:
class Builder final {
@@ -240,7 +240,7 @@
fOutPoint,
fDuration;
- typedef SkNVRefCnt<Animation> INHERITED;
+ typedef SkRefCnt INHERITED;
};
} // namespace skottie
diff --git a/src/gpu/GrBackendTextureImageGenerator.h b/src/gpu/GrBackendTextureImageGenerator.h
index 9d76e02..5f1afae 100644
--- a/src/gpu/GrBackendTextureImageGenerator.h
+++ b/src/gpu/GrBackendTextureImageGenerator.h
@@ -50,7 +50,7 @@
static void ReleaseRefHelper_TextureReleaseProc(void* ctx);
- class RefHelper : public SkNVRefCnt<RefHelper> {
+ class RefHelper : public SkRefCnt {
public:
RefHelper(GrTexture* texture, uint32_t owningContextID)
: fOriginalTexture(texture)
diff --git a/src/gpu/effects/GrSkSLFP.h b/src/gpu/effects/GrSkSLFP.h
index c4acfd0..619c568 100644
--- a/src/gpu/effects/GrSkSLFP.h
+++ b/src/gpu/effects/GrSkSLFP.h
@@ -126,7 +126,7 @@
* upon the inputs to the SkSL (static if's, etc.) we first create a factory for a given SkSL
* string, then use that to create the actual GrFragmentProcessor.
*/
-class GrSkSLFPFactory : public SkNVRefCnt<GrSkSLFPFactory> {
+class GrSkSLFPFactory : public SkRefCnt {
public:
/**
* Constructs a GrSkSLFPFactory for a given SkSL source string. Creating a factory will
diff --git a/src/gpu/text/GrDistanceFieldAdjustTable.h b/src/gpu/text/GrDistanceFieldAdjustTable.h
index fca9a8c..03e4bab 100644
--- a/src/gpu/text/GrDistanceFieldAdjustTable.h
+++ b/src/gpu/text/GrDistanceFieldAdjustTable.h
@@ -14,7 +14,7 @@
// Distance field text needs this table to compute a value for use in the fragment shader.
// Because the GrTextContext can go out of scope before the final flush, this needs to be
// refcnted and malloced
-struct GrDistanceFieldAdjustTable : public SkNVRefCnt<GrDistanceFieldAdjustTable> {
+struct GrDistanceFieldAdjustTable : public SkRefCnt {
GrDistanceFieldAdjustTable() { this->buildDistanceAdjustTables(); }
~GrDistanceFieldAdjustTable() {
delete[] fTable;
diff --git a/src/gpu/text/GrGlyphCache.h b/src/gpu/text/GrGlyphCache.h
index c38ba78..ec4ca2b 100644
--- a/src/gpu/text/GrGlyphCache.h
+++ b/src/gpu/text/GrGlyphCache.h
@@ -26,7 +26,7 @@
* of it's SkDescriptor as a key to access (or regenerate) the SkGlyphCache. GrTextStrikes are
* created by and owned by a GrGlyphCache.
*/
-class GrTextStrike : public SkNVRefCnt<GrTextStrike> {
+class GrTextStrike : public SkRefCnt {
public:
GrTextStrike(const SkDescriptor& fontScalerKey);
~GrTextStrike();
diff --git a/src/gpu/text/GrTextBlob.h b/src/gpu/text/GrTextBlob.h
index 8451d48..c5f34cd 100644
--- a/src/gpu/text/GrTextBlob.h
+++ b/src/gpu/text/GrTextBlob.h
@@ -47,7 +47,7 @@
*
* *WARNING* If you add new fields to this struct, then you may need to to update AssertEqual
*/
-class GrTextBlob : public SkNVRefCnt<GrTextBlob> {
+class GrTextBlob : public SkRefCnt {
public:
SK_DECLARE_INTERNAL_LLIST_INTERFACE(GrTextBlob);
diff --git a/src/image/SkImage_Lazy.cpp b/src/image/SkImage_Lazy.cpp
index 2107251..b5f72e3 100644
--- a/src/image/SkImage_Lazy.cpp
+++ b/src/image/SkImage_Lazy.cpp
@@ -28,7 +28,7 @@
#endif
// Ref-counted tuple(SkImageGenerator, SkMutex) which allows sharing one generator among N images
-class SharedGenerator final : public SkNVRefCnt<SharedGenerator> {
+class SharedGenerator final : public SkRefCnt {
public:
static sk_sp<SharedGenerator> Make(std::unique_ptr<SkImageGenerator> gen) {
return gen ? sk_sp<SharedGenerator>(new SharedGenerator(std::move(gen))) : nullptr;