begin refactoring SkTDynamicHash and SkTMultiMap

The biggest mismatch between these and SkTHash{Map,Set,Table}
is the old ones provide Iter and ConstIter, the new ones foreach().

This CL,

   - adds foreach() methods to the old types,
   - replaces all uses of ConstIter with foreach(),
   - replaces most uses of Iter with foreach(),

I'm leaving one spot using Iter to walk the table and remove its
elements for its own CL... it'll be a little more complicated to get
that right.

From there it should be straightforward to turn SkTDynamicHash
into a thin wrapper over an SkTHashTable.

Bugs: skia:9703
Change-Id: Ia6ba87c35b89585c42b5b9f118f4cbf3abd04f0d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/277098
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
This commit is contained in:
Mike Klein 2020-03-14 16:22:45 -05:00 committed by Skia Commit-Bot
parent 16701eecd5
commit cff6396875
12 changed files with 108 additions and 198 deletions

View File

@ -151,6 +151,7 @@ public:
// around to check slot i again.
}
}
// TODO: shrink table capacity like in remove()?
}
private:

View File

@ -32,13 +32,7 @@ public:
typedef SkImageFilterCacheKey Key;
CacheImpl(size_t maxBytes) : fMaxBytes(maxBytes), fCurrentBytes(0) { }
~CacheImpl() override {
SkTDynamicHash<Value, Key>::Iter iter(&fLookup);
while (!iter.done()) {
Value* v = &*iter;
++iter;
delete v;
}
fLookup.foreach([&](Value* v) { delete v; });
}
struct Value {
Value(const Key& key, const skif::FilterResult<For::kOutput>& image,

View File

@ -30,59 +30,21 @@ public:
sk_free(fArray);
}
class Iter {
public:
explicit Iter(SkTDynamicHash* hash) : fHash(hash), fCurrentIndex(-1) {
SkASSERT(hash);
++(*this);
}
bool done() const {
SkASSERT(fCurrentIndex <= fHash->fCapacity);
return fCurrentIndex == fHash->fCapacity;
}
T& operator*() const {
SkASSERT(!this->done());
return *this->current();
}
void operator++() {
do {
fCurrentIndex++;
} while (!this->done() && (this->current() == Empty() || this->current() == Deleted()));
// Call fn on every entry in the table. You may mutate the entries, but be very careful.
template <typename Fn> // f(T*)
void foreach(Fn&& fn) {
for (Iter it(this); !it.done(); ++it) {
fn(&*it);
}
}
private:
T* current() const { return fHash->fArray[fCurrentIndex]; }
SkTDynamicHash* fHash;
int fCurrentIndex;
};
class ConstIter {
public:
explicit ConstIter(const SkTDynamicHash* hash) : fHash(hash), fCurrentIndex(-1) {
SkASSERT(hash);
++(*this);
// Call fn on every entry in the table. You may not mutate anything.
template <typename Fn> // f(T) or f(const T&)
void foreach(Fn&& fn) const {
for (ConstIter it(this); !it.done(); ++it) {
fn(*it);
}
bool done() const {
SkASSERT(fCurrentIndex <= fHash->fCapacity);
return fCurrentIndex == fHash->fCapacity;
}
const T& operator*() const {
SkASSERT(!this->done());
return *this->current();
}
void operator++() {
do {
fCurrentIndex++;
} while (!this->done() && (this->current() == Empty() || this->current() == Deleted()));
}
private:
const T* current() const { return fHash->fArray[fCurrentIndex]; }
const SkTDynamicHash* fHash;
int fCurrentIndex;
};
}
int count() const { return fCount; }
@ -160,6 +122,66 @@ private:
static T* Empty() { return reinterpret_cast<T*>(0); } // i.e. nullptr
static T* Deleted() { return reinterpret_cast<T*>(1); } // Also an invalid pointer.
// GrProxyProvider::removeAllUniqueKeys() uses Iter but removes elements from the table
// as it iterates, so it can't safely use foreach().
// TODO(mtklein): add mutate(), replace that use of Iter with that.
friend class GrProxyProvider;
class Iter {
public:
explicit Iter(SkTDynamicHash* hash) : fHash(hash), fCurrentIndex(-1) {
SkASSERT(hash);
++(*this);
}
bool done() const {
SkASSERT(fCurrentIndex <= fHash->fCapacity);
return fCurrentIndex == fHash->fCapacity;
}
T& operator*() const {
SkASSERT(!this->done());
return *this->current();
}
void operator++() {
do {
fCurrentIndex++;
} while (!this->done() && (this->current() == Empty() || this->current() == Deleted()));
}
private:
T* current() const { return fHash->fArray[fCurrentIndex]; }
SkTDynamicHash* fHash;
int fCurrentIndex;
};
class ConstIter {
public:
explicit ConstIter(const SkTDynamicHash* hash) : fHash(hash), fCurrentIndex(-1) {
SkASSERT(hash);
++(*this);
}
bool done() const {
SkASSERT(fCurrentIndex <= fHash->fCapacity);
return fCurrentIndex == fHash->fCapacity;
}
const T& operator*() const {
SkASSERT(!this->done());
return *this->current();
}
void operator++() {
do {
fCurrentIndex++;
} while (!this->done() && (this->current() == Empty() || this->current() == Deleted()));
}
private:
const T* current() const { return fHash->fArray[fCurrentIndex]; }
const SkTDynamicHash* fHash;
int fCurrentIndex;
};
bool validate() const {
#define SKTDYNAMICHASH_CHECK(x) SkASSERT(x); if (!(x)) return false
static const int kLarge = 50; // Arbitrary, tweak to suit your patience.

View File

@ -30,15 +30,14 @@ public:
SkTMultiMap() : fCount(0) {}
~SkTMultiMap() {
typename SkTDynamicHash<ValueList, Key>::Iter iter(&fHash);
for ( ; !iter.done(); ++iter) {
fHash.foreach([&](ValueList* vl) {
ValueList* next;
for (ValueList* cur = &(*iter); cur; cur = next) {
HashTraits::OnFree(cur->fValue);
next = cur->fNext;
delete cur;
for (ValueList* it = vl; it; it = next) {
HashTraits::OnFree(it->fValue);
next = it->fNext;
delete it;
}
}
});
}
void insert(const Key& key, T* value) {
@ -127,41 +126,14 @@ public:
int count() const { return fCount; }
#ifdef SK_DEBUG
class ConstIter {
public:
explicit ConstIter(const SkTMultiMap* mmap)
: fIter(&(mmap->fHash))
, fList(nullptr) {
if (!fIter.done()) {
fList = &(*fIter);
template <typename Fn> // f(T) or f(const T&)
void foreach(Fn&& fn) const {
fHash.foreach([&](const ValueList& vl) {
for (const ValueList* it = &vl; it; it = it->fNext) {
fn(*it->fValue);
}
}
bool done() const {
return fIter.done();
}
const T* operator*() {
SkASSERT(fList);
return fList->fValue;
}
void operator++() {
if (fList) {
fList = fList->fNext;
}
if (!fList) {
++fIter;
if (!fIter.done()) {
fList = &(*fIter);
}
}
}
private:
typename SkTDynamicHash<ValueList, Key>::ConstIter fIter;
const ValueList* fList;
};
});
}
bool has(const T* value, const Key& key) const {
for (ValueList* list = fHash.find(key); list; list = list->fNext) {

View File

@ -891,12 +891,9 @@ bool GrProxyProvider::isAbandoned() const {
}
void GrProxyProvider::orphanAllUniqueKeys() {
UniquelyKeyedProxyHash::Iter iter(&fUniquelyKeyedProxies);
for (UniquelyKeyedProxyHash::Iter iter(&fUniquelyKeyedProxies); !iter.done(); ++iter) {
GrTextureProxy& tmp = *iter;
tmp.fProxyProvider = nullptr;
}
fUniquelyKeyedProxies.foreach([&](GrTextureProxy* proxy){
proxy->fProxyProvider = nullptr;
});
}
void GrProxyProvider::removeAllUniqueKeys() {

View File

@ -863,16 +863,13 @@ void GrResourceCache::validate() const {
};
{
ScratchMap::ConstIter iter(&fScratchMap);
int count = 0;
for ( ; !iter.done(); ++iter) {
const GrGpuResource* resource = *iter;
SkASSERT(resource->resourcePriv().getScratchKey().isValid());
SkASSERT(!resource->getUniqueKey().isValid());
fScratchMap.foreach([&](const GrGpuResource& resource) {
SkASSERT(resource.resourcePriv().getScratchKey().isValid());
SkASSERT(!resource.getUniqueKey().isValid());
count++;
}
SkASSERT(count == fScratchMap.count()); // ensure the iterator is working correctly
});
SkASSERT(count == fScratchMap.count());
}
Stats stats(this);

View File

@ -70,18 +70,10 @@ GrMtlSampler* GrMtlResourceProvider::findOrCreateCompatibleSampler(GrSamplerStat
}
void GrMtlResourceProvider::destroyResources() {
// Iterate through all stored GrMtlSamplers and unref them before resetting the hash.
SkTDynamicHash<GrMtlSampler, GrMtlSampler::Key>::Iter samplerIter(&fSamplers);
for (; !samplerIter.done(); ++samplerIter) {
(*samplerIter).unref();
}
fSamplers.foreach([&](GrMtlSampler* sampler) { sampler->unref(); });
fSamplers.reset();
// Iterate through all stored GrMtlDepthStencils and unref them before resetting the hash.
SkTDynamicHash<GrMtlDepthStencil, GrMtlDepthStencil::Key>::Iter dsIter(&fDepthStencilStates);
for (; !dsIter.done(); ++dsIter) {
(*dsIter).unref();
}
fDepthStencilStates.foreach([&](GrMtlDepthStencil* stencil) { stencil->unref(); });
fDepthStencilStates.reset();
fPipelineStateCache->release();

View File

@ -392,14 +392,10 @@ void GrVkResourceProvider::destroyResources(bool deviceLost) {
fExternalRenderPasses.reset();
// Iterate through all store GrVkSamplers and unref them before resetting the hash.
for (decltype(fSamplers)::Iter iter(&fSamplers); !iter.done(); ++iter) {
(*iter).unref();
}
fSamplers.foreach([&](auto* elt) { elt->unref(); });
fSamplers.reset();
for (decltype(fYcbcrConversions)::Iter iter(&fYcbcrConversions); !iter.done(); ++iter) {
(*iter).unref();
}
fYcbcrConversions.foreach([&](auto* elt) { elt->unref(); });
fYcbcrConversions.reset();
fPipelineStateCache->release();

View File

@ -171,59 +171,6 @@ DEF_TEST(DynamicHash_remove, reporter) {
ASSERT(hash.find(5)->value == 3.0);
}
template<typename T> static void TestIter(skiatest::Reporter* reporter) {
Hash hash;
int count = 0;
// this should fall out of loop immediately
for (T iter(&hash); !iter.done(); ++iter) {
++count;
}
ASSERT(0 == count);
// These collide.
Entry a = { 1, 2.0 };
Entry b = { 5, 3.0 };
Entry c = { 9, 4.0 };
hash.add(&a);
hash.add(&b);
hash.add(&c);
// should see all 3 unique keys when iterating over hash
count = 0;
int keys[3] = {0, 0, 0};
for (T iter(&hash); !iter.done(); ++iter) {
int key = (*iter).key;
keys[count] = key;
ASSERT(hash.find(key) != nullptr);
++count;
}
ASSERT(3 == count);
ASSERT(keys[0] != keys[1]);
ASSERT(keys[0] != keys[2]);
ASSERT(keys[1] != keys[2]);
// should see 2 unique keys when iterating over hash that aren't 1
hash.remove(1);
count = 0;
memset(keys, 0, sizeof(keys));
for (T iter(&hash); !iter.done(); ++iter) {
int key = (*iter).key;
keys[count] = key;
ASSERT(key != 1);
ASSERT(hash.find(key) != nullptr);
++count;
}
ASSERT(2 == count);
ASSERT(keys[0] != keys[1]);
}
DEF_TEST(DynamicHash_iterator, reporter) {
TestIter<Hash::Iter>(reporter);
TestIter<Hash::ConstIter>(reporter);
}
static void TestResetOrRewind(skiatest::Reporter* reporter, bool testReset) {
Hash hash;
Entry a = { 1, 2.0 };

View File

@ -35,13 +35,9 @@ SkString UrlDataManager::addData(SkData* data, const char* contentType) {
}
void UrlDataManager::reset() {
SkTDynamicHash<UrlData, SkData, LookupTrait>::Iter iter(&fCache);
while (!iter.done()) {
UrlData* urlData = &(*iter);
fCache.foreach([&](UrlData* urlData) {
urlData->unref();
++iter;
}
});
fCache.rewind();
}

View File

@ -43,13 +43,11 @@ void GrResourceCache::changeTimestamp(uint32_t newTimestamp) { fTimestamp = newT
#ifdef SK_DEBUG
int GrResourceCache::countUniqueKeysWithTag(const char* tag) const {
int count = 0;
UniqueHash::ConstIter iter(&fUniqueHash);
while (!iter.done()) {
if (0 == strcmp(tag, (*iter).getUniqueKey().tag())) {
fUniqueHash.foreach([&](const GrGpuResource& resource){
if (0 == strcmp(tag, resource.getUniqueKey().tag())) {
++count;
}
++iter;
}
});
return count;
}
#endif

View File

@ -148,13 +148,11 @@ bool Window_mac::attach(BackendType attachType) {
}
void Window_mac::PaintWindows() {
SkTDynamicHash<Window_mac, NSInteger>::Iter iter(&gWindowMap);
while (!iter.done()) {
if ((*iter).fIsContentInvalidated) {
(*iter).onPaint();
gWindowMap.foreach([&](Window_mac* window) {
if (window->fIsContentInvalidated) {
window->onPaint();
}
++iter;
}
});
}
} // namespace sk_app