Revert "Clean up SkString reference counting a bit."
This reverts commit a910c847e9d04e183e9e610902cbd363c8488196.
Reason for revert: Compilation failure on Ubuntu14 bots
../../../../../work/skia/src/core/SkString.cpp:200:55: error: could not convert ‘{0, {0}, 0}’ from ‘<brace-enclosed initializer list>’ to ‘const SkString::Rec’
const SkString::Rec SkString::gEmptyRec = { 0, {0}, 0 };
Original change's description:
> Clean up SkString reference counting a bit.
>
> BUG=skia:7107
>
> Change-Id: I47072bf31b902c79dbb850179ff6d35940de3e63
> Reviewed-on: https://skia-review.googlesource.com/54720
> Reviewed-by: Ben Wagner <bungeman@google.com>
> Commit-Queue: Ben Wagner <bungeman@google.com>
TBR=mtklein@google.com,bungeman@google.com,reed@google.com
Change-Id: I6ec327511e8e1c1fd7e4c1bd5839c0547d4ab609
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:7107
Reviewed-on: https://skia-review.googlesource.com/55640
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
diff --git a/src/core/SkString.cpp b/src/core/SkString.cpp
index 23ef8f1..d0e3c12 100644
--- a/src/core/SkString.cpp
+++ b/src/core/SkString.cpp
@@ -220,7 +220,7 @@
return extra;
}
-sk_sp<SkString::Rec> SkString::Rec::Make(const char text[], size_t len) {
+SkString::Rec* SkString::AllocRec(const char text[], size_t len) {
Rec* rec;
if (0 == len) {
@@ -237,29 +237,15 @@
}
rec->data()[len] = 0;
}
- return sk_sp<SkString::Rec>(rec);
+ return rec;
}
-void SkString::Rec::ref() const {
- if (this == &SkString::gEmptyRec) {
- return;
+SkString::Rec* SkString::RefRec(Rec* src) {
+ if (src != &gEmptyRec) {
+ // No barrier required.
+ (void)src->fRefCnt.fetch_add(+1, std::memory_order_relaxed);
}
- // No barrier required.
- (void)this->fRefCnt.fetch_add(+1, std::memory_order_relaxed);
-}
-
-void SkString::Rec::unref() const {
- if (this == &SkString::gEmptyRec) {
- return;
- }
- SkASSERT(this->fRefCnt.load(std::memory_order_relaxed) > 0);
- if (1 == this->fRefCnt.fetch_add(-1, std::memory_order_acq_rel)) {
- sk_free(const_cast<SkString::Rec*>(this));
- }
-}
-
-bool SkString::Rec::unique() const {
- return fRefCnt.load(std::memory_order_acquire) == 1;
+ return src;
}
#ifdef SK_DEBUG
@@ -269,7 +255,7 @@
SkASSERT(0 == gEmptyRec.fRefCnt.load(std::memory_order_relaxed));
SkASSERT(0 == gEmptyRec.data()[0]);
- if (fRec.get() != &gEmptyRec) {
+ if (fRec != &gEmptyRec) {
SkASSERT(fRec->fLength > 0);
SkASSERT(fRec->fRefCnt.load(std::memory_order_relaxed) > 0);
SkASSERT(0 == fRec->data()[fRec->fLength]);
@@ -283,34 +269,41 @@
}
SkString::SkString(size_t len) {
- fRec = Rec::Make(nullptr, len);
+ fRec = AllocRec(nullptr, len);
}
SkString::SkString(const char text[]) {
size_t len = text ? strlen(text) : 0;
- fRec = Rec::Make(text, len);
+ fRec = AllocRec(text, len);
}
SkString::SkString(const char text[], size_t len) {
- fRec = Rec::Make(text, len);
+ fRec = AllocRec(text, len);
}
SkString::SkString(const SkString& src) {
src.validate();
- fRec = src.fRec;
+ fRec = RefRec(src.fRec);
}
SkString::SkString(SkString&& src) {
src.validate();
- fRec = std::move(src.fRec);
- src.fRec.reset(const_cast<Rec*>(&gEmptyRec));
+ fRec = src.fRec;
+ src.fRec = const_cast<Rec*>(&gEmptyRec);
}
SkString::~SkString() {
this->validate();
+
+ if (fRec->fLength) {
+ SkASSERT(fRec->fRefCnt.load(std::memory_order_relaxed) > 0);
+ if (1 == fRec->fRefCnt.fetch_add(-1, std::memory_order_acq_rel)) {
+ sk_free(fRec);
+ }
+ }
}
bool SkString::equals(const SkString& src) const {
@@ -357,7 +350,15 @@
void SkString::reset() {
this->validate();
- fRec.reset(const_cast<Rec*>(&gEmptyRec));
+
+ if (fRec->fLength) {
+ SkASSERT(fRec->fRefCnt.load(std::memory_order_relaxed) > 0);
+ if (1 == fRec->fRefCnt.fetch_add(-1, std::memory_order_acq_rel)) {
+ sk_free(fRec);
+ }
+ }
+
+ fRec = const_cast<Rec*>(&gEmptyRec);
}
char* SkString::writable_str() {
@@ -365,7 +366,14 @@
if (fRec->fLength) {
if (!fRec->unique()) {
- fRec = Rec::Make(fRec->data(), fRec->fLength);
+ Rec* rec = AllocRec(fRec->data(), fRec->fLength);
+ if (1 == fRec->fRefCnt.fetch_add(-1, std::memory_order_acq_rel)) {
+ // In this case after our check of fRecCnt > 1, we suddenly
+ // did become the only owner, so now we have two copies of the
+ // data (fRec and rec), so we need to delete one of them.
+ sk_free(fRec);
+ }
+ fRec = rec;
}
}
return fRec->data();
@@ -625,7 +633,7 @@
this->validate();
other.validate();
- SkTSwap(fRec, other.fRec);
+ SkTSwap<Rec*>(fRec, other.fRec);
}
///////////////////////////////////////////////////////////////////////////////