Refactored GrDLinkedList into SkTDLinkedList
authorrobertphillips@google.com <robertphillips@google.com@2bbb7eff-a529-9590-31e7-b0007b416f81>
Thu, 23 Aug 2012 11:13:48 +0000 (11:13 +0000)
committerrobertphillips@google.com <robertphillips@google.com@2bbb7eff-a529-9590-31e7-b0007b416f81>
Thu, 23 Aug 2012 11:13:48 +0000 (11:13 +0000)
http://codereview.appspot.com/6484045/

git-svn-id: http://skia.googlecode.com/svn/trunk@5247 2bbb7eff-a529-9590-31e7-b0007b416f81

gyp/core.gypi
gyp/tests.gyp
include/core/SkTDLinkedList.h [new file with mode: 0644]
src/gpu/GrResourceCache.cpp
src/gpu/GrResourceCache.h
tests/TDLinkedListTest.cpp [new file with mode: 0644]

index 3a36987e5e5e54aa81b5a697ac6623ef2294216b..4595b6ae1c69b065ed6b3521ea7e4e07d17440da 100644 (file)
         '<(skia_include_path)/core/SkTDArray.h',
         '<(skia_include_path)/core/SkTDStack.h',
         '<(skia_include_path)/core/SkTDict.h',
+        '<(skia_include_path)/core/SkTDLinkedList.h',
         '<(skia_include_path)/core/SkTRegistry.h',
         '<(skia_include_path)/core/SkTScopedPtr.h',
         '<(skia_include_path)/core/SkTSearch.h',
index fa04dcecd47260b6c0e6bc911addc41059d6eb79..170250e64835089f92ba86bfe9ef65bbf533d9da 100644 (file)
@@ -83,6 +83,7 @@
         '../tests/SrcOverTest.cpp',
         '../tests/StreamTest.cpp',
         '../tests/StringTest.cpp',
+        '../tests/TDLinkedListTest.cpp',
         '../tests/Test.cpp',
         '../tests/Test.h',
         '../tests/TestSize.cpp',
diff --git a/include/core/SkTDLinkedList.h b/include/core/SkTDLinkedList.h
new file mode 100644 (file)
index 0000000..5dfad55
--- /dev/null
@@ -0,0 +1,175 @@
+/*
+ * Copyright 2012 Google Inc.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#ifndef SkTDLinkedList_DEFINED
+#define SkTDLinkedList_DEFINED
+
+#include "SkTypes.h"
+
+/** 
+ * Helper class to automatically initialize the doubly linked list
+ * created pointers.
+ */
+template <typename T> class SkPtrWrapper {
+  public:
+      SkPtrWrapper() : fPtr(NULL) {}
+      SkPtrWrapper& operator =(T* ptr) { fPtr = ptr; return *this; }
+      operator T*() const { return fPtr; }
+      T* operator->() { return fPtr; }
+  private:
+      T* fPtr;
+};
+
+
+/**
+ * This macro creates the member variables required by
+ * the SkTDLinkedList class. It should be placed in the private section
+ * of any class that will be stored in a double linked list.
+ */
+#define SK_DEFINE_DLINKEDLIST_INTERFACE(ClassName)              \
+    friend class SkTDLinkedList<ClassName>;                     \
+    /* back pointer to the owning list - for debugging */       \
+    SkDEBUGCODE(SkPtrWrapper<SkTDLinkedList<ClassName> > fList;)\
+    SkPtrWrapper<ClassName> fPrev;                              \
+    SkPtrWrapper<ClassName> fNext;
+
+/**
+ * This class implements a templated internal doubly linked list data structure.
+ */
+template <class T> class SkTDLinkedList : public SkNoncopyable {
+public:
+    SkTDLinkedList()
+        : fHead(NULL)
+        , fTail(NULL) {
+    }
+
+    void remove(T* entry) {
+        SkASSERT(this->isInList(entry));
+
+        T* prev = entry->fPrev;
+        T* next = entry->fNext;
+
+        if (NULL != prev) {
+            prev->fNext = next;
+        } else {
+            fHead = next;
+        }
+        if (NULL != next) {
+            next->fPrev = prev;
+        } else {
+            fTail = prev;
+        }
+
+        entry->fPrev = NULL;
+        entry->fNext = NULL;
+
+#if SK_DEBUG
+        entry->fList = NULL;
+#endif
+    }
+
+    void addToHead(T* entry) {
+        SkASSERT(NULL == entry->fPrev && NULL == entry->fNext);
+        SkASSERT(NULL == entry->fList);
+
+        entry->fPrev = NULL;
+        entry->fNext = fHead;
+        if (NULL != fHead) {
+            fHead->fPrev = entry;
+        }
+        fHead = entry;
+        if (NULL == fTail) {
+            fTail = entry;
+        }
+
+#if SK_DEBUG
+        entry->fList = this;
+#endif
+    }
+
+    bool isEmpty() const { 
+        return NULL == fHead && NULL == fTail; 
+    }
+
+    class Iter {
+    public:
+        enum IterStart {
+            kHead_IterStart,
+            kTail_IterStart
+        };
+
+        Iter() : fCur(NULL) {}
+
+        T* init(SkTDLinkedList& list, IterStart startLoc) {
+            if (kHead_IterStart == startLoc) {
+                fCur = list.fHead;
+            } else {
+                SkASSERT(kTail_IterStart == startLoc);
+                fCur = list.fTail;
+            }
+
+            return fCur;
+        }
+
+        /**
+         * Return the next/previous element in the list or NULL if at the end.
+         */
+        T* next() {
+            if (NULL == fCur) {
+                return NULL;
+            }
+
+            fCur = fCur->fNext;
+            return fCur;
+        }
+
+        T* prev() {
+            if (NULL == fCur) {
+                return NULL;
+            }
+
+            fCur = fCur->fPrev;
+            return fCur;
+        }
+
+    private:
+        T* fCur;
+    };
+
+#if SK_DEBUG
+    void validate() const {
+        GrAssert(!fHead == !fTail);
+    }
+
+    /**
+     * Debugging-only method that uses the list back pointer to check if
+     * 'entry' is indeed in 'this' list.
+     */
+    bool isInList(const T* entry) const {
+        return entry->fList == this;
+    }
+
+    /**
+     * Debugging-only method that laboriously counts the list entries.
+     */
+    int countEntries() const {
+        int count = 0;
+        for (T* entry = fHead; NULL != entry; entry = entry->fNext) {
+            ++count;
+        }
+        return count;
+    }
+#endif // SK_DEBUG
+
+private:
+    T* fHead;
+    T* fTail;
+
+    typedef SkNoncopyable INHERITED;
+};
+
+#endif // SkTDLinkedList_DEFINED
index be796247abb769ce0c30648876c4923e8aa3fd81..cae2ec9bbac57af282df71dea58b06ae01121914 100644 (file)
@@ -14,7 +14,6 @@
 GrResourceEntry::GrResourceEntry(const GrResourceKey& key, GrResource* resource)
         : fKey(key), fResource(resource) {
     fLockCount = 0;
-    fPrev = fNext = NULL;
 
     // we assume ownership of the resource, and will unref it when we die
     GrAssert(resource);
@@ -311,7 +310,15 @@ void GrResourceCache::purgeAsNeeded() {
         fPurging = true;
         bool withinBudget = false;
         do {
-            GrResourceEntry* entry = fList.fTail;
+            SkTDLinkedList<GrResourceEntry>::Iter iter;
+            
+            // Note: the following code relies on the fact that the 
+            // doubly linked list doesn't invalidate its data/pointers
+            // outside of the specific area where a deletion occurs (e.g.,
+            // in internalDetach)
+            GrResourceEntry* entry = iter.init(fList,
+                    SkTDLinkedList<GrResourceEntry>::Iter::kTail_IterStart);
+
             while (entry && fUnlockedEntryCount) {
                 GrAutoResourceCacheValidate atcv(this);
                 if (fEntryCount <= fMaxCount && fEntryBytes <= fMaxBytes) {
@@ -319,7 +326,7 @@ void GrResourceCache::purgeAsNeeded() {
                     break;
                 }
 
-                GrResourceEntry* prev = entry->fPrev;
+                GrResourceEntry* prev = iter.prev();
                 if (!entry->isLocked()) {
                     // remove from our cache
                     fCache.remove(entry->fKey, entry);
@@ -358,7 +365,7 @@ void GrResourceCache::removeAll() {
 
 #if GR_DEBUG
     GrAssert(fExclusiveList.countEntries() == fClientDetachedCount);
-    GrAssert(fExclusiveList.countBytes() == fClientDetachedBytes);
+    GrAssert(countBytes(fExclusiveList) == fClientDetachedBytes);
     GrAssert(!fUnlockedEntryCount);
     if (!fCache.count()) {
         // Items may have been detached from the cache (such as the backing
@@ -377,16 +384,19 @@ void GrResourceCache::removeAll() {
 ///////////////////////////////////////////////////////////////////////////////
 
 #if GR_DEBUG
-static int countMatches(const GrResourceEntry* head, const GrResourceEntry* target) {
-    const GrResourceEntry* entry = head;
-    int count = 0;
-    while (entry) {
-        if (target == entry) {
-            count += 1;
-        }
-        entry = entry->next();
+size_t GrResourceCache::countBytes(const SkTDLinkedList<GrResourceEntry>& list) {
+    size_t bytes = 0;
+
+    SkTDLinkedList<GrResourceEntry>::Iter iter;
+    
+    const GrResourceEntry* entry = iter.init(
+                  const_cast<SkTDLinkedList<GrResourceEntry>&>(list),
+                  SkTDLinkedList<GrResourceEntry>::Iter::kTail_IterStart);
+
+    for ( ; NULL != entry; entry = iter.prev()) {
+        bytes += entry->resource()->sizeInBytes();
     }
-    return count;
+    return bytes;
 }
 
 static bool both_zero_or_nonzero(int count, size_t bytes) {
@@ -394,7 +404,8 @@ static bool both_zero_or_nonzero(int count, size_t bytes) {
 }
 
 void GrResourceCache::validate() const {
-    GrAssert(!fList.fHead == !fList.fTail);
+    fList.validate();
+    fExclusiveList.validate();
     GrAssert(both_zero_or_nonzero(fEntryCount, fEntryBytes));
     GrAssert(both_zero_or_nonzero(fClientDetachedCount, fClientDetachedBytes));
     GrAssert(fClientDetachedBytes <= fEntryBytes);
@@ -403,24 +414,29 @@ void GrResourceCache::validate() const {
 
     fCache.validate();
 
-    GrResourceEntry* entry = fList.fHead;
     int count = 0;
     int unlockCount = 0;
-    while (entry) {
+
+    SkTDLinkedList<GrResourceEntry>::Iter iter;
+    
+    const GrResourceEntry* entry = iter.init(
+                  const_cast<SkTDLinkedList<GrResourceEntry>&>(fList),
+                  SkTDLinkedList<GrResourceEntry>::Iter::kHead_IterStart);
+
+    for ( ; NULL != entry; entry = iter.next()) {
         entry->validate();
         GrAssert(fCache.find(entry->key()));
         count += 1;
         if (!entry->isLocked()) {
             unlockCount += 1;
         }
-        entry = entry->fNext;
     }
     GrAssert(count == fEntryCount - fClientDetachedCount);
 
-    size_t bytes = fList.countBytes();
+    size_t bytes = countBytes(fList);
     GrAssert(bytes == fEntryBytes  - fClientDetachedBytes);
 
-    bytes = fExclusiveList.countBytes();
+    bytes = countBytes(fExclusiveList);
     GrAssert(bytes == fClientDetachedBytes);
 
     GrAssert(unlockCount == fUnlockedEntryCount);
@@ -428,11 +444,6 @@ void GrResourceCache::validate() const {
     GrAssert(fList.countEntries() == fEntryCount - fClientDetachedCount);
 
     GrAssert(fExclusiveList.countEntries() == fClientDetachedCount);
-
-    for (int i = 0; i < count; i++) {
-        int matches = countMatches(fList.fHead, fCache.getArray()[i]);
-        GrAssert(1 == matches);
-    }
 }
 
 void GrResourceCache::printStats() const {
@@ -452,68 +463,3 @@ void GrResourceCache::printStats() const {
 #endif
 
 ///////////////////////////////////////////////////////////////////////////////
-void GrDLinkedList::remove(GrResourceEntry* entry) {
-    GrAssert(this->isInList(entry));
-
-    GrResourceEntry* prev = entry->fPrev;
-    GrResourceEntry* next = entry->fNext;
-
-    if (prev) {
-        prev->fNext = next;
-    } else {
-        fHead = next;
-    }
-    if (next) {
-        next->fPrev = prev;
-    } else {
-        fTail = prev;
-    }
-
-    entry->fPrev = NULL;
-    entry->fNext = NULL;
-}
-
-void GrDLinkedList::addToHead(GrResourceEntry* entry) {
-    GrAssert(NULL == entry->fPrev && NULL == entry->fNext);
-
-    entry->fPrev = NULL;
-    entry->fNext = fHead;
-    if (fHead) {
-        fHead->fPrev = entry;
-    }
-    fHead = entry;
-    if (NULL == fTail) {
-        fTail = entry;
-    }
-}
-
-#if GR_DEBUG
-
-bool GrDLinkedList::isInList(const GrResourceEntry* entry) const {
-
-    for (GrResourceEntry* cur = fHead; NULL != cur; cur = cur->fNext) {
-        if (entry == cur) {
-            return true;
-        }
-    }
-
-    return false;
-}
-
-int GrDLinkedList::countEntries() const {
-    int count = 0;
-    for (GrResourceEntry* entry = fTail; NULL != entry; entry = entry->fPrev) {
-        ++count;
-    }
-    return count;
-}
-
-size_t GrDLinkedList::countBytes() const {
-    size_t bytes = 0;
-    for (GrResourceEntry* entry = fTail; NULL != entry; entry = entry->fPrev) {
-        bytes += entry->resource()->sizeInBytes();
-    }
-    return bytes;
-}
-
-#endif // GR_DEBUG
index 7b5870e250099cbf9dc6d66880c969a4ca168075..f2b6602389a124e1d00d11d7d75e63318777d46f 100644 (file)
@@ -13,6 +13,7 @@
 
 #include "GrTypes.h"
 #include "GrTHashCache.h"
+#include "SkTDLinkedList.h"
 
 class GrResource;
 
@@ -123,11 +124,6 @@ public:
     GrResource* resource() const { return fResource; }
     const GrResourceKey& key() const { return fKey; }
 
-#if GR_DEBUG
-    GrResourceEntry* next() const { return fNext; }
-    GrResourceEntry* prev() const { return fPrev; }
-#endif
-
 #if GR_DEBUG
     void validate() const;
 #else
@@ -153,8 +149,7 @@ private:
     int fLockCount;
 
     // we're a dlinklist
-    GrResourceEntry* fPrev;
-    GrResourceEntry* fNext;
+    SK_DEFINE_DLINKEDLIST_INTERFACE(GrResourceEntry);
 
     friend class GrResourceCache;
     friend class GrDLinkedList;
@@ -164,27 +159,6 @@ private:
 
 #include "GrTHashCache.h"
 
-class GrDLinkedList {
-public:
-    GrDLinkedList()
-        : fHead(NULL)
-        , fTail(NULL) {
-    }
-
-    void remove(GrResourceEntry* entry);
-    void addToHead(GrResourceEntry* entry);
-
-#ifdef GR_DEBUG
-    bool isInList(const GrResourceEntry* entry) const;
-    bool isEmpty() const { return NULL == fHead && NULL == fTail; }
-    int countEntries() const;
-    size_t countBytes() const;
-#endif
-
-    GrResourceEntry* fHead;
-    GrResourceEntry* fTail;
-};
-
 /**
  *  Cache of GrResource objects.
  *
@@ -315,11 +289,11 @@ private:
     GrTHashTable<GrResourceEntry, Key, 8> fCache;
 
     // manage the dlink list
-    GrDLinkedList    fList;
+    SkTDLinkedList<GrResourceEntry>    fList;
 
 #if GR_DEBUG
     // These objects cannot be returned by a search
-    GrDLinkedList    fExclusiveList;
+    SkTDLinkedList<GrResourceEntry>    fExclusiveList;
 #endif
 
     // our budget, used in purgeAsNeeded()
@@ -349,6 +323,10 @@ private:
                             bool lock,
                             bool clientReattach);
 
+#if GR_DEBUG
+    static size_t GrResourceCache::countBytes(
+                        const SkTDLinkedList<GrResourceEntry>& list);
+#endif
 };
 
 ///////////////////////////////////////////////////////////////////////////////
diff --git a/tests/TDLinkedListTest.cpp b/tests/TDLinkedListTest.cpp
new file mode 100644 (file)
index 0000000..8df39b8
--- /dev/null
@@ -0,0 +1,89 @@
+/*
+ * Copyright 2012 Google Inc.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#include "Test.h"
+#include "SkTDLinkedList.h"
+
+class ListElement {
+public:
+    ListElement(int id) : fID(id) {
+    }
+
+    int fID;
+
+private:
+    SK_DEFINE_DLINKEDLIST_INTERFACE(ListElement);
+};
+
+static void CheckList(const SkTDLinkedList<ListElement>& list,
+                      skiatest::Reporter* reporter,
+                      bool empty,
+                      int numElements,
+                      bool in0, bool in1, bool in2, bool in3,
+                      ListElement elements[4]) {
+
+    REPORTER_ASSERT(reporter, empty == list.isEmpty());
+#if SK_DEBUG
+    REPORTER_ASSERT(reporter, numElements == list.countEntries());
+    REPORTER_ASSERT(reporter, in0 == list.isInList(&elements[0]));
+    REPORTER_ASSERT(reporter, in1 == list.isInList(&elements[1]));
+    REPORTER_ASSERT(reporter, in2 == list.isInList(&elements[2]));
+    REPORTER_ASSERT(reporter, in3 == list.isInList(&elements[3]));
+#endif
+}
+
+static void TestTDLinkedList(skiatest::Reporter* reporter) {
+    SkTDLinkedList<ListElement> list;
+    ListElement elements[4] = {
+        ListElement(0),
+        ListElement(1),
+        ListElement(2),
+        ListElement(3),
+    };
+
+    // list should be empty to start with
+    CheckList(list, reporter, true, 0, false, false, false, false, elements);
+
+    list.addToHead(&elements[0]);
+
+    CheckList(list, reporter, false, 1, true, false, false, false, elements);
+
+    list.addToHead(&elements[1]);
+    list.addToHead(&elements[2]);
+    list.addToHead(&elements[3]);
+
+    CheckList(list, reporter, false, 4, true, true, true, true, elements);
+
+    // test out iterators
+    SkTDLinkedList<ListElement>::Iter iter;
+
+    ListElement* cur = iter.init(list, SkTDLinkedList<ListElement>::Iter::kHead_IterStart);
+    for (int i = 0; NULL != cur; ++i, cur = iter.next()) {
+        REPORTER_ASSERT(reporter, cur->fID == 3-i);
+    }
+
+    cur = iter.init(list, SkTDLinkedList<ListElement>::Iter::kTail_IterStart);
+    for (int i = 0; NULL != cur; ++i, cur = iter.prev()) {
+        REPORTER_ASSERT(reporter, cur->fID == i);
+    }
+
+    // remove middle, frontmost then backmost
+    list.remove(&elements[1]);
+    list.remove(&elements[3]);
+    list.remove(&elements[0]);
+
+    CheckList(list, reporter, false, 1, false, false, true, false, elements);
+
+    // remove last element
+    list.remove(&elements[2]);
+
+    // list should be empty again
+    CheckList(list, reporter, true, 0, false, false, false, false, elements);
+}
+
+#include "TestClassDef.h"
+DEFINE_TESTCLASS("TDLinkedList", TestTDLinkedListClass, TestTDLinkedList)