add move semantics to SkTHash*
The more I look at std::unordered_map and co., the less I like them.
I think we might want to bet on SkTHash*.
As a simple first improvement, add move support.
Next comes shrinking, and then I'll start moving over SkTDynamicHash users.
BUG=skia:6053
Change-Id: Ifdb5d713aab66434ca271c7f18a0cbbb0720099c
Reviewed-on: https://skia-review.googlesource.com/5943
Commit-Queue: Mike Klein <mtklein@chromium.org>
Reviewed-by: Herb Derby <herb@google.com>
Reviewed-by: Hal Canary <halcanary@google.com>
diff --git a/gn/BUILD.gn b/gn/BUILD.gn
index 88427c0..a52690d 100644
--- a/gn/BUILD.gn
+++ b/gn/BUILD.gn
@@ -283,6 +283,7 @@
"-Wvla",
"-Wno-deprecated-declarations",
+ "-Wno-maybe-uninitialized",
]
cflags_cc += [ "-Wnon-virtual-dtor" ]
diff --git a/include/private/SkTHash.h b/include/private/SkTHash.h
index 8a644e3..333f653 100644
--- a/include/private/SkTHash.h
+++ b/include/private/SkTHash.h
@@ -50,12 +50,13 @@
// Copy val into the hash table, returning a pointer to the copy now in the table.
// If there already is an entry in the table with the same key, we overwrite it.
- T* set(const T& val) {
+ T* set(T&& val) {
if (4 * (fCount+fRemoved) >= 3 * fCapacity) {
this->resize(fCapacity > 0 ? fCapacity * 2 : 4);
}
- return this->uncheckedSet(val);
+ return this->uncheckedSet(std::move(val));
}
+ T* set(const T& val) { return this->set(std::move(T(val))); }
// If there is an entry in the table with this key, return a pointer to it. If not, NULL.
T* find(const K& key) const {
@@ -116,7 +117,7 @@
}
private:
- T* uncheckedSet(const T& val) {
+ T* uncheckedSet(T&& val) {
const K& key = Traits::GetKey(val);
uint32_t hash = Hash(key);
int index = hash & (fCapacity-1);
@@ -127,7 +128,7 @@
if (s.removed()) {
fRemoved--;
}
- s.val = val;
+ s.val = std::move(val);
s.hash = hash;
fCount++;
return &s.val;
@@ -135,7 +136,7 @@
if (hash == s.hash && key == Traits::GetKey(s.val)) {
// Overwrite previous entry.
// Note: this triggers extra copies when adding the same value repeatedly.
- s.val = val;
+ s.val = std::move(val);
return &s.val;
}
index = this->next(index, n);
@@ -154,9 +155,9 @@
oldSlots.swap(fSlots);
for (int i = 0; i < oldCapacity; i++) {
- const Slot& s = oldSlots[i];
+ Slot& s = oldSlots[i];
if (!s.empty() && !s.removed()) {
- this->uncheckedSet(s.val);
+ this->uncheckedSet(std::move(s.val));
}
}
SkASSERT(fCount == oldCount);
@@ -209,11 +210,11 @@
// Set key to val in the table, replacing any previous value with the same key.
// We copy both key and val, and return a pointer to the value copy now in the table.
- V* set(const K& key, const V& val) {
- Pair in = { key, val };
- Pair* out = fTable.set(in);
+ V* set(K&& key, V&& val) {
+ Pair* out = fTable.set({std::move(key), std::move(val)});
return &out->val;
}
+ V* set(const K& key, const V& val) { return this->set(std::move(K(key)), std::move(V(val))); }
// If there is key/value entry in the table with this key, return a pointer to the value.
// If not, return NULL.
@@ -269,7 +270,8 @@
size_t approxBytesUsed() const { return fTable.approxBytesUsed(); }
// Copy an item into the set.
- void add(const T& item) { fTable.set(item); }
+ void add(T&& item) { fTable.set(std::move(item)); }
+ void add(const T& item) { this->add(std::move(T(item))); }
// Is this item in the set?
bool contains(const T& item) const { return SkToBool(this->find(item)); }
diff --git a/tests/HashTest.cpp b/tests/HashTest.cpp
index c9b1bc9..4a884e3 100644
--- a/tests/HashTest.cpp
+++ b/tests/HashTest.cpp
@@ -111,6 +111,13 @@
*fCounter += 1;
}
+ CopyCounter(CopyCounter&& other) { *this = std::move(other); }
+ void operator=(CopyCounter&& other) {
+ fID = other.fID;
+ fCounter = other.fCounter;
+ }
+
+
bool operator==(const CopyCounter& other) const {
return fID == other.fID;
}