Update SkString::resize to preserve string contents efficiently.

Without this change, the following unit test failures would occur:

StringTest.cpp:425     [String_resize_grow]
StringTest.cpp:435     [String_resize_after_assignment]
StringTest.cpp:438     [String_resize_after_assignment]
StringTest.cpp:444     [String_resize_after_copy_construction]
StringTest.cpp:446     [String_resize_after_copy_construction]

Change-Id: Ib4f63d51604e55d32f1049136b733ee905b72039
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/298217
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Mike Reed <reed@google.com>
Reviewed-by: Ben Wagner <bungeman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This commit is contained in:
John Stiles 2020-06-22 17:54:05 -04:00 committed by Skia Commit-Bot
parent 6dd62cbdfa
commit b444943db2
3 changed files with 74 additions and 17 deletions

View File

@ -181,10 +181,10 @@ public:
char& operator[](size_t n) { return this->writable_str()[n]; }
void reset();
/** Destructive resize, does not preserve contents.
/** String contents are preserved on resize. (For destructive resize, `set(nullptr, length)`.)
* `resize` automatically reserves an extra byte at the end of the buffer for a null terminator.
*/
void resize(size_t len) { this->set(nullptr, len); }
void resize(size_t len);
void set(const SkString& src) { *this = src; }
void set(const char text[]);
void set(const char text[], size_t len);

View File

@ -42,9 +42,9 @@ static StringBuffer apply_format_string(const char* format, va_list args, char (
}
// Our text was too long to fit on the stack! However, we now know how much space we need to
// format it. Format the string into our heap buffer. `resize` automatically reserves an extra
// format it. Format the string into our heap buffer. `set` automatically reserves an extra
// byte at the end of the buffer for a null terminator, so we don't need to add one here.
heapBuffer->resize(outLength);
heapBuffer->set(nullptr, outLength);
char* heapBufferDest = heapBuffer->writable_str();
SkDEBUGCODE(int checkLength =) std::vsnprintf(heapBufferDest, outLength + 1, format, argsCopy);
SkASSERT(checkLength == outLength);
@ -327,31 +327,40 @@ char* SkString::writable_str() {
return fRec->data();
}
void SkString::resize(size_t len) {
len = trim_size_t_to_u32(len);
if (0 == len) {
this->reset();
} else if (fRec->unique() && ((len >> 2) <= (fRec->fLength >> 2))) {
// Use less of the buffer we have without allocating a smaller one.
char* p = this->writable_str();
p[len] = '\0';
fRec->fLength = SkToU32(len);
} else {
SkString newString(len);
char* dest = newString.writable_str();
int copyLen = std::min<uint32_t>(len, this->size());
memcpy(dest, this->c_str(), copyLen);
dest[copyLen] = '\0';
this->swap(newString);
}
}
void SkString::set(const char text[]) {
this->set(text, text ? strlen(text) : 0);
}
void SkString::set(const char text[], size_t len) {
len = trim_size_t_to_u32(len);
bool unique = fRec->unique();
if (0 == len) {
this->reset();
} else if (unique && len <= fRec->fLength) {
// should we resize if len <<<< fLength, to save RAM? (e.g. len < (fLength>>1))?
// just use less of the buffer without allocating a smaller one
} else if (fRec->unique() && ((len >> 2) <= (fRec->fLength >> 2))) {
// Use less of the buffer we have without allocating a smaller one.
char* p = this->writable_str();
if (text) {
memcpy(p, text, len);
}
p[len] = 0;
fRec->fLength = SkToU32(len);
} else if (unique && (fRec->fLength >> 2) == (len >> 2)) {
// we have spare room in the current allocation, so don't alloc a larger one
char* p = this->writable_str();
if (text) {
memcpy(p, text, len);
}
p[len] = 0;
p[len] = '\0';
fRec->fLength = SkToU32(len);
} else {
SkString tmp(text, len);

View File

@ -403,3 +403,51 @@ DEF_TEST(String_VAList_overflow, r) {
test_va_list_overflow_append(r, "%1999s", " ");
test_va_list_overflow_prepend(r, "%1999s", " ");
}
DEF_TEST(String_resize_to_nothing, r) {
SkString s("hello world!");
REPORTER_ASSERT(r, s.equals("hello world!"));
s.resize(0);
REPORTER_ASSERT(r, s.equals(""));
}
DEF_TEST(String_resize_shrink, r) {
SkString s("hello world!");
REPORTER_ASSERT(r, s.equals("hello world!"));
s.resize(5);
REPORTER_ASSERT(r, s.equals("hello"));
}
DEF_TEST(String_resize_grow, r) {
SkString s("hello world!");
REPORTER_ASSERT(r, s.equals("hello world!"));
s.resize(25);
REPORTER_ASSERT(r, 0 == strcmp(s.c_str(), "hello world!")); // no promises about data past \0
REPORTER_ASSERT(r, s.size() == 25);
}
DEF_TEST(String_resize_after_assignment, r) {
SkString s("hello world!");
SkString t;
t = s;
REPORTER_ASSERT(r, s.equals("hello world!"));
s.resize(25);
REPORTER_ASSERT(r, 0 == strcmp(s.c_str(), "hello world!"));
REPORTER_ASSERT(r, s.size() == 25);
s.resize(5);
REPORTER_ASSERT(r, s.equals("hello"));
}
static void resize_helper_function(skiatest::Reporter* r, SkString s) {
REPORTER_ASSERT(r, s.equals("hello world!"));
s.resize(5);
REPORTER_ASSERT(r, s.equals("hello"));
s.resize(25);
REPORTER_ASSERT(r, 0 == strcmp(s.c_str(), "hello"));
REPORTER_ASSERT(r, s.size() == 25);
}
DEF_TEST(String_resize_after_copy_construction, r) {
SkString s("hello world!");
resize_helper_function(r, s);
}