Revert "Clean up SkString reference counting a bit."

This reverts commit a910c847e9.

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>
This commit is contained in:
Robert Phillips 2017-10-05 11:28:20 +00:00 committed by Skia Commit-Bot
parent 1ea81e766e
commit 01f8e41c13
2 changed files with 44 additions and 39 deletions

View File

@ -12,7 +12,6 @@
#include "../private/SkTArray.h"
#include "SkScalar.h"
#include "SkRefCnt.h"
#include <atomic>
#include <stdarg.h>
@ -241,20 +240,16 @@ public:
private:
struct Rec {
public:
Rec() = delete;
static sk_sp<Rec> Make(const char text[], size_t len);
uint32_t fLength; // logically size_t, but we want it to stay 32bits
mutable std::atomic<int32_t> fRefCnt;
std::atomic<int32_t> fRefCnt;
char fBeginningOfData;
char* data() { return &fBeginningOfData; }
const char* data() const { return &fBeginningOfData; }
void ref() const;
void unref() const;
bool unique() const;
bool unique() { return fRefCnt.load(std::memory_order_acquire) == 1; }
};
sk_sp<Rec> fRec;
Rec* fRec;
#ifdef SK_DEBUG
void validate() const;
@ -263,6 +258,8 @@ private:
#endif
static const Rec gEmptyRec;
static Rec* AllocRec(const char text[], size_t len);
static Rec* RefRec(Rec*);
};
/// Creates a new string and writes into it using a printf()-style format.

View File

@ -220,7 +220,7 @@ static size_t check_add32(size_t base, size_t extra) {
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 @@ sk_sp<SkString::Rec> SkString::Rec::Make(const char text[], size_t len) {
}
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 @@ void SkString::validate() const {
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() : fRec(const_cast<Rec*>(&gEmptyRec)) {
}
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 @@ SkString& SkString::operator=(const char text[]) {
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 @@ char* SkString::writable_str() {
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 @@ void SkString::swap(SkString& other) {
this->validate();
other.validate();
SkTSwap(fRec, other.fRec);
SkTSwap<Rec*>(fRec, other.fRec);
}
///////////////////////////////////////////////////////////////////////////////