Revert "replace SkNVRefCnt with SkRefCnt"
This reverts commit 0fb1ee98cf7dfb87a3b376de8e0dff28c1b95619.
Reason for revert: looks like this increased size by ~8K.
Original change's description:
> 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>
TBR=mtklein@google.com,bsalomon@google.com,mtklein@chromium.org
Change-Id: Ibcfcc4b523c466a535bea5ffa30d0fe2574c5bd7
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/166360
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
diff --git a/include/core/SkColorSpace.h b/include/core/SkColorSpace.h
index 5ca9b45..26771f3 100644
--- a/include/core/SkColorSpace.h
+++ b/include/core/SkColorSpace.h
@@ -62,7 +62,7 @@
float fF;
};
-class SK_API SkColorSpace : public SkRefCnt {
+class SK_API SkColorSpace : public SkNVRefCnt<SkColorSpace> {
public:
/**
* Create the sRGB color space.
diff --git a/include/core/SkData.h b/include/core/SkData.h
index c279d52..931749a 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 SkRefCnt {
+class SK_API SkData final : public SkNVRefCnt<SkData> {
public:
/**
* Returns the number of bytes stored.
@@ -158,7 +158,7 @@
static sk_sp<SkData> MakeEmpty();
private:
- friend class SkRefCnt;
+ friend class SkNVRefCnt<SkData>;
ReleaseProc fReleaseProc;
void* fReleaseProcContext;
void* fPtr;
diff --git a/include/core/SkRefCnt.h b/include/core/SkRefCnt.h
index ee2710d..bc6ccd8 100644
--- a/include/core/SkRefCnt.h
+++ b/include/core/SkRefCnt.h
@@ -163,6 +163,44 @@
}
}
+///////////////////////////////////////////////////////////////////////////////
+
+// 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 81c44fc..907241c 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 SkRefCnt {
+class SK_API SkTextBlob final : public SkNVRefCnt<SkTextBlob> {
public:
/** Returns conservative bounding box. Uses SkPaint associated with each glyph to
@@ -138,7 +138,7 @@
const SkDeserialProcs& procs);
private:
- friend class SkRefCnt;
+ friend class SkNVRefCnt<SkTextBlob>;
class RunRecord;
enum GlyphPositioning : uint8_t;
diff --git a/include/core/SkVertices.h b/include/core/SkVertices.h
index f36a877..63a27a0 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 SkRefCnt {
+class SK_API SkVertices : public SkNVRefCnt<SkVertices> {
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 SkRefCnt;
+ friend class SkNVRefCnt<SkVertices>;
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 7440791..40e001a 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 SkRefCnt {
+class GrSkSLFPFactoryCache : public SkNVRefCnt<GrSkSLFPFactoryCache> {
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 630fdfb..d250fc3 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 SkRefCnt {
+class SK_API SkPathRef final : public SkNVRefCnt<SkPathRef> {
public:
class Editor {
public: