Reland "Saves 20K: Move non-parametric code for SkTDArray to common code"
This is a reland of commit e529a4637f677a9463c26d87f78330ec5483e7e4
Add SK_SPI to get the symbols to show up.
Original change's description:
> Saves 20K: Move non-parametric code for SkTDArray to common code
>
> Share common code from each of the template instances.
> The big wins in sharing:
> resizeStorageToAtLeast
> append
> redoing the constructors
>
> Savings:
> From bloaty:
> FILE SIZE VM SIZE
> -------------- --------------
> +100% -128 +100% -128 .rodata
> +99% -1.44Ki +99% -1.44Ki .eh_frame_hdr
> +65% -2.76Ki [ = ] 0 [Unmapped]
> +99% -6.34Ki +99% -6.34Ki .eh_frame
> +100% -9.34Ki +100% -9.34Ki .text
>
> Total -20Ki
>
> Stripped binary difference:
> 6502952 -> 6482472 = -20480
>
> Bug: skia:13657
> Change-Id: I71c629d815411c75452b51de6f086e11de62a105
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/580577
> Reviewed-by: John Stiles <johnstiles@google.com>
> Commit-Queue: Herb Derby <herb@google.com>
> Reviewed-by: Brian Salomon <bsalomon@google.com>
Bug: skia:13657
Change-Id: Ieeedceaf20e603b387089b20af426a15b5309a24
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/583242
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Herb Derby <herb@google.com>
diff --git a/BUILD.gn b/BUILD.gn
index 2271a19..1074036 100644
--- a/BUILD.gn
+++ b/BUILD.gn
@@ -1562,6 +1562,7 @@
"src/core/SkStroke.cpp",
"src/core/SkStrokeRec.cpp",
"src/core/SkStrokerPriv.cpp",
+ "src/core/SkTDArray.cpp",
"src/core/SkThreadID.cpp",
"src/core/SkUtils.cpp",
"src/effects/SkDashPathEffect.cpp",
diff --git a/gn/core.gni b/gn/core.gni
index 017f882..b42ff4e 100644
--- a/gn/core.gni
+++ b/gn/core.gni
@@ -422,6 +422,7 @@
"$_src/core/SkSurfacePriv.h",
"$_src/core/SkSwizzle.cpp",
"$_src/core/SkTBlockList.h",
+ "$_src/core/SkTDArray.cpp",
"$_src/core/SkTDPQueue.h",
"$_src/core/SkTDynamicHash.h",
"$_src/core/SkTInternalLList.h",
diff --git a/include/private/SkTDArray.h b/include/private/SkTDArray.h
index 997a240..d92e324 100644
--- a/include/private/SkTDArray.h
+++ b/include/private/SkTDArray.h
@@ -18,6 +18,35 @@
#include <tuple>
#include <utility>
+class SK_SPI SkTDStorage {
+public:
+ SkTDStorage() = default;
+ SkTDStorage(const SkTDStorage& that) = delete;
+ SkTDStorage& operator= (const SkTDStorage& that) = delete;
+ SkTDStorage(SkTDStorage&& that);
+ SkTDStorage& operator= (SkTDStorage&& that);
+ ~SkTDStorage();
+
+ int assign(const void* src, int count, size_t sizeOfT);
+
+ int resizeStorageToAtLeast(int count, size_t sizeOfT);
+ int shrinkToFit(int count, size_t sizeOfT);
+ void swap(SkTDStorage& that) {
+ using std::swap;
+ swap(fStorage, that.fStorage);
+ }
+ template <typename T>
+ T* data() const { return static_cast<T*>(fStorage); }
+
+ struct StateUpdate {int count, reserve;};
+ StateUpdate append(
+ const void* src, int count, size_t sizeOfT, int reserve, int oldCount);
+private:
+ void* fStorage{nullptr};
+};
+
+template <typename T> static inline void swap(SkTDStorage& a, SkTDStorage& b) { a.swap(b); }
+
// SkTDArray<T> implements a std::vector-like array for raw data-only objects that do not require
// construction or destruction. The constructor and destructor for T will not be called; T objects
// will always be moved via raw memcpy. Newly created T objects will contain uninitialized memory.
@@ -26,55 +55,52 @@
// used with appropriate care. In new code, consider std::vector<T> instead.
template <typename T> class SkTDArray {
public:
- SkTDArray() : fArray(nullptr), fReserve(0), fCount(0) {}
+ SkTDArray() = default;
SkTDArray(const T src[], int count) {
SkASSERT(src || count == 0);
-
- fReserve = fCount = 0;
- fArray = nullptr;
- if (count) {
- fArray = (T*)sk_malloc_throw(SkToSizeT(count) * sizeof(T));
- memcpy(fArray, src, sizeof(T) * SkToSizeT(count));
- fReserve = fCount = count;
- }
+ fReserve = fStorage.assign(src, count, sizeof(T));
+ fCount = count;
}
SkTDArray(const std::initializer_list<T>& list) : SkTDArray(list.begin(), list.size()) {}
- SkTDArray(const SkTDArray<T>& src) : fArray(nullptr), fReserve(0), fCount(0) {
- SkTDArray<T> tmp(src.fArray, src.fCount);
- this->swap(tmp);
+ SkTDArray(const SkTDArray<T>& src) {
+ fReserve = fStorage.assign(src.array(), src.fCount, sizeof(T));
+ fCount = src.fCount;
}
- SkTDArray(SkTDArray<T>&& src) : fArray(nullptr), fReserve(0), fCount(0) { this->swap(src); }
- ~SkTDArray() { sk_free(fArray); }
-
SkTDArray<T>& operator=(const SkTDArray<T>& src) {
if (this != &src) {
if (src.fCount > fReserve) {
- SkTDArray<T> tmp(src.fArray, src.fCount);
- this->swap(tmp);
+ fReserve = fStorage.assign(src.array(), src.fCount, sizeof(T));
} else {
- sk_careful_memcpy(fArray, src.fArray, sizeof(T) * SkToSizeT(src.fCount));
- fCount = src.fCount;
+ sk_careful_memcpy(this->array(), src.array(), sizeof(T) * SkToSizeT(src.fCount));
}
+ fCount = src.fCount;
}
return *this;
}
+
+ SkTDArray(SkTDArray<T>&& src)
+ : fStorage{std::move(src.fStorage)}
+ , fReserve{src.fReserve}
+ , fCount{src.fCount} {}
+
SkTDArray<T>& operator=(SkTDArray<T>&& src) {
if (this != &src) {
- this->swap(src);
- src.reset();
+ fStorage = std::move(src.fStorage);
+ fReserve = std::exchange(src.fReserve, 0);
+ fCount = std::exchange(src.fCount, 0);
}
return *this;
}
friend bool operator==(const SkTDArray<T>& a, const SkTDArray<T>& b) {
return a.fCount == b.fCount &&
- (a.fCount == 0 || !memcmp(a.fArray, b.fArray, SkToSizeT(a.fCount) * sizeof(T)));
+ (a.fCount == 0 || !memcmp(a.array(), b.array(), SkToSizeT(a.fCount) * sizeof(T)));
}
friend bool operator!=(const SkTDArray<T>& a, const SkTDArray<T>& b) { return !(a == b); }
void swap(SkTDArray<T>& that) {
using std::swap;
- swap(fArray, that.fArray);
+ swap(fStorage, that.fStorage);
swap(fReserve, that.fReserve);
swap(fCount, that.fCount);
}
@@ -94,41 +120,36 @@
// return the number of bytes in the array: count * sizeof(T)
size_t bytes() const { return fCount * sizeof(T); }
- T* data() { return fArray; }
- const T* data() const { return fArray; }
- T* begin() { return fArray; }
- const T* begin() const { return fArray; }
- T* end() { return fArray ? fArray + fCount : nullptr; }
- const T* end() const { return fArray ? fArray + fCount : nullptr; }
+ T* data() { return this->array(); }
+ const T* data() const { return this->array(); }
+ T* begin() { return this->array(); }
+ const T* begin() const { return this->array(); }
+ T* end() { return this->array() ? this->array() + fCount : nullptr; }
+ const T* end() const { return this->array() ? this->array() + fCount : nullptr; }
T& operator[](int index) {
SkASSERT(index < fCount);
- return fArray[index];
+ return this->array()[index];
}
const T& operator[](int index) const {
SkASSERT(index < fCount);
- return fArray[index];
+ return this->array()[index];
}
T& getAt(int index) { return (*this)[index]; }
const T& back() const {
SkASSERT(fCount > 0);
- return fArray[fCount - 1];
+ return this->array()[fCount - 1];
}
T& back() {
SkASSERT(fCount > 0);
- return fArray[fCount - 1];
+ return this->array()[fCount - 1];
}
void reset() {
- if (fArray) {
- sk_free(fArray);
- fArray = nullptr;
- fReserve = fCount = 0;
- } else {
- SkASSERT(fReserve == 0 && fCount == 0);
- }
+ this->~SkTDArray();
+ new (this) SkTDArray{};
}
void rewind() {
@@ -161,23 +182,17 @@
T* prepend() {
this->adjustCount(1);
- memmove(fArray + 1, fArray, (fCount - 1) * sizeof(T));
- return fArray;
+ memmove(this->array() + 1, this->array(), (fCount - 1) * sizeof(T));
+ return this->array();
}
T* append() { return this->append(1, nullptr); }
T* append(int count, const T* src = nullptr) {
int oldCount = fCount;
- if (count) {
- SkASSERT(src == nullptr || fArray == nullptr || src + count <= fArray ||
- fArray + oldCount <= src);
-
- this->adjustCount(count);
- if (src) {
- memcpy(fArray + oldCount, src, sizeof(T) * count);
- }
- }
- return fArray + oldCount;
+ auto [newCount, newReserve] = fStorage.append(src, count, sizeof(T), fReserve, fCount);
+ fCount = newCount;
+ fReserve = newReserve;
+ return this->array() + oldCount;
}
T* insert(int index) { return this->insert(index, 1, nullptr); }
@@ -186,7 +201,7 @@
SkASSERT(index <= fCount);
size_t oldCount = fCount;
this->adjustCount(count);
- T* dst = fArray + index;
+ T* dst = this->array() + index;
memmove(dst + count, dst, sizeof(T) * (oldCount - index));
if (src) {
memcpy(dst, src, sizeof(T) * count);
@@ -197,7 +212,7 @@
void remove(int index, int count = 1) {
SkASSERT(index + count <= fCount);
fCount = fCount - count;
- memmove(fArray + index, fArray + index + count, sizeof(T) * (fCount - index));
+ memmove(this->array() + index, this->array() + index + count, sizeof(T) * (fCount - index));
}
void removeShuffle(int index) {
@@ -205,25 +220,25 @@
int newCount = fCount - 1;
fCount = newCount;
if (index != newCount) {
- memcpy(fArray + index, fArray + newCount, sizeof(T));
+ memcpy(this->array() + index, this->array() + newCount, sizeof(T));
}
}
int find(const T& elem) const {
- const T* iter = fArray;
- const T* stop = fArray + fCount;
+ const T* iter = this->array();
+ const T* stop = this->array() + fCount;
for (; iter < stop; iter++) {
if (*iter == elem) {
- return SkToInt(iter - fArray);
+ return SkToInt(iter - this->array());
}
}
return -1;
}
int rfind(const T& elem) const {
- const T* iter = fArray + fCount;
- const T* stop = fArray;
+ const T* iter = this->array() + fCount;
+ const T* stop = this->array();
while (iter > stop) {
if (*--iter == elem) {
@@ -245,7 +260,7 @@
return 0;
}
int count = std::min(max, fCount - index);
- memcpy(dst, fArray + index, sizeof(T) * count);
+ memcpy(dst, this->array() + index, sizeof(T) * count);
return count;
}
@@ -267,8 +282,8 @@
}
void deleteAll() {
- T* iter = fArray;
- T* stop = fArray + fCount;
+ T* iter = this->array();
+ T* stop = this->array() + fCount;
while (iter < stop) {
delete *iter;
iter += 1;
@@ -277,8 +292,8 @@
}
void freeAll() {
- T* iter = fArray;
- T* stop = fArray + fCount;
+ T* iter = this->array();
+ T* stop = this->array() + fCount;
while (iter < stop) {
sk_free(*iter);
iter += 1;
@@ -287,8 +302,8 @@
}
void unrefAll() {
- T* iter = fArray;
- T* stop = fArray + fCount;
+ T* iter = this->array();
+ T* stop = this->array() + fCount;
while (iter < stop) {
(*iter)->unref();
iter += 1;
@@ -297,8 +312,8 @@
}
void safeUnrefAll() {
- T* iter = fArray;
- T* stop = fArray + fCount;
+ T* iter = this->array();
+ T* stop = this->array() + fCount;
while (iter < stop) {
SkSafeUnref(*iter);
iter += 1;
@@ -308,20 +323,22 @@
#ifdef SK_DEBUG
void validate() const {
- SkASSERT((fReserve == 0 && fArray == nullptr) || (fReserve > 0 && fArray != nullptr));
+ SkASSERT((fReserve == 0 && this->array() == nullptr) ||
+ (fReserve > 0 && this->array() != nullptr));
SkASSERT(fCount <= fReserve);
}
#endif
void shrinkToFit() {
- if (fReserve != fCount) {
- SkASSERT(fReserve > fCount);
- fReserve = fCount;
- fArray = (T*)sk_realloc_throw(fArray, fReserve * sizeof(T));
+ if (fReserve > fCount) {
+ fReserve = fStorage.shrinkToFit(fCount, sizeof(T));
}
}
private:
+ T* array() { return fStorage.data<T>(); }
+ const T* array() const { return fStorage.data<T>(); }
+
// Adjusts the number of elements in the array.
// This is the same as calling setCount(count() + delta).
void adjustCount(int delta) {
@@ -335,28 +352,6 @@
this->setCount(SkTo<int>(count));
}
- static std::tuple<void*, int> ResizeStorageToAtLeast(void* array, int count, size_t TSize) {
- // Establish the maximum number of elements that includes a valid count for end. In the
- // largest case end() = &fArray[INT_MAX] which is 1 after the last indexable element.
- static constexpr int kMaxCount = INT_MAX;
-
- // Assume that the array will max out.
- int newReserve = kMaxCount;
- if (kMaxCount - count > 4) {
- // Add 1/4 more than we need. Add 4 to ensure this grows by at least 1. Pin to
- // kMaxCount if no room for 1/4 growth.
- int growth = 4 + ((count + 4) >> 2);
- // Read this line as: if (count + growth < kMaxCount) { ... }
- // It's rewritten to avoid signed integer overflow.
- if (kMaxCount - count > growth) {
- newReserve = count + growth;
- }
- }
-
- void* newArray = sk_realloc_throw(array, SkToSizeT(newReserve) * TSize);
- return {newArray, newReserve};
- }
-
// Increase the storage allocation such that it can hold (fCount + extra)
// elements.
// It never shrinks the allocation, and it may increase the allocation by
@@ -365,15 +360,12 @@
// note: this does NOT modify fCount
void resizeStorageToAtLeast(int count) {
SkASSERT(count > fReserve);
-
- auto [array, reserve] = ResizeStorageToAtLeast(fArray, count, sizeof(T));
- fArray = static_cast<T*>(array);
- fReserve = reserve;
+ fReserve = fStorage.resizeStorageToAtLeast(count, sizeof(T));
}
- T* fArray;
- int fReserve; // size of the allocation in fArray (#elements)
- int fCount; // logical number of elements (fCount <= fReserve)
+ SkTDStorage fStorage;
+ int fReserve = 0; // size of the allocation in fArray (#elements)
+ int fCount = 0; // logical number of elements (fCount <= fReserve)
};
template <typename T> static inline void swap(SkTDArray<T>& a, SkTDArray<T>& b) { a.swap(b); }
diff --git a/public.bzl b/public.bzl
index 83803c2..ef586c4 100644
--- a/public.bzl
+++ b/public.bzl
@@ -639,6 +639,7 @@
"src/core/SkSurfacePriv.h",
"src/core/SkSwizzle.cpp",
"src/core/SkTBlockList.h",
+ "src/core/SkTDArray.cpp",
"src/core/SkTDPQueue.h",
"src/core/SkTDynamicHash.h",
"src/core/SkTInternalLList.h",
diff --git a/src/core/BUILD.bazel b/src/core/BUILD.bazel
index d22716b..97b8efb 100644
--- a/src/core/BUILD.bazel
+++ b/src/core/BUILD.bazel
@@ -329,6 +329,7 @@
"SkSurfacePriv.h",
"SkSwizzle.cpp",
"SkTBlockList.h",
+ "SkTDArray.cpp",
"SkTDPQueue.h",
"SkTDynamicHash.h",
"SkTInternalLList.h",
diff --git a/src/core/SkTDArray.cpp b/src/core/SkTDArray.cpp
new file mode 100644
index 0000000..aeac67a
--- /dev/null
+++ b/src/core/SkTDArray.cpp
@@ -0,0 +1,82 @@
+/*
+ * Copyright 2018 Google Inc.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#include "include/private/SkTDArray.h"
+#include "include/private/SkTo.h"
+
+SkTDStorage::SkTDStorage(SkTDStorage&& that)
+ : fStorage{std::move(that.fStorage)} { that.fStorage = nullptr; }
+
+SkTDStorage& SkTDStorage::operator=(SkTDStorage&& that) {
+ if (this != &that) {
+ this->~SkTDStorage();
+ new (this) SkTDStorage{std::move(that)};
+ }
+ return *this;
+}
+
+SkTDStorage::~SkTDStorage() {
+ sk_free(fStorage);
+}
+
+int SkTDStorage::assign(const void* src, int count, size_t sizeOfT) {
+ if (count > 0) {
+ fStorage = sk_realloc_throw(fStorage, SkToSizeT(count) * sizeOfT);
+ memcpy(fStorage, src, SkToSizeT(count) * sizeOfT);
+ }
+ return count;
+}
+
+int SkTDStorage::resizeStorageToAtLeast(int count, size_t sizeOfT) {
+ SkASSERT(count > 0);
+ // Establish the maximum number of elements that includes a valid count for end. In the
+ // largest case end() = &fArray[INT_MAX] which is 1 after the last indexable element.
+ static constexpr int kMaxCount = INT_MAX;
+
+ // Assume that the array will max out.
+ int newReserve = kMaxCount;
+ if (kMaxCount - count > 4) {
+ // Add 1/4 more than we need. Add 4 to ensure this grows by at least 1. Pin to
+ // kMaxCount if no room for 1/4 growth.
+ int growth = 4 + ((count + 4) >> 2);
+ // Read this line as: if (count + growth < kMaxCount) { ... }
+ // It's rewritten to avoid signed integer overflow.
+ if (kMaxCount - count > growth) {
+ newReserve = count + growth;
+ }
+ }
+
+ fStorage = sk_realloc_throw(fStorage, SkToSizeT(newReserve) * sizeOfT);
+ return newReserve;
+}
+
+int SkTDStorage::shrinkToFit(int count, size_t sizeOfT) {
+ fStorage = sk_realloc_throw(fStorage, SkToSizeT(count) * sizeOfT);
+ return count;
+}
+
+SkTDStorage::StateUpdate SkTDStorage::append(
+ const void* src, int count, size_t sizeOfT, int reserve, int oldCount) {
+ SkASSERT(count >= 0);
+ int newCount = oldCount;
+ int newReserve = reserve;
+ if (count > 0) {
+ // We take care to avoid overflow here.
+ // The sum of fCount and delta is at most 4294967294, which fits fine in uint32_t.
+ uint32_t testCount = (uint32_t)oldCount + (uint32_t)count;
+ SkASSERT_RELEASE(SkTFitsIn<int>(testCount));
+ newCount = testCount;
+ if (newCount > reserve) {
+ newReserve = this->resizeStorageToAtLeast(newCount, sizeOfT);
+ }
+ if (src != nullptr) {
+ memcpy(this->data<char>() + sizeOfT *SkToSizeT(oldCount), src,
+ sizeOfT *SkToSizeT(count));
+ }
+ }
+ return {newCount, newReserve};
+}
