Big Cleanup: SkBitmapFactory, SkLazyPixelRef, SkImageCache

include/lazy/SkDiscardableMemoryPool.h

+/*
+ * Copyright 2013 Google Inc.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#ifndef SkDiscardableMemoryPool_DEFINED
+#define SkDiscardableMemoryPool_DEFINED
+
+#include "SkDiscardableMemory.h"
+#include "SkTInternalLList.h"
+
+class SkBaseMutex;
+
+#ifdef SK_DEBUG
+    #define LAZY_CACHE_STATS 1
+#elif !defined(LAZY_CACHE_STATS)
+    #define LAZY_CACHE_STATS 0
+#endif
+
+/**
+ *  This non-global pool can be used for unit tests to verify that the

[scroggo, 2013/12/03 23:00:01]

If this is just for unit tests, does it need to be in include?

[hal.canary, 2013/12/04 00:01:36]

Done.

Mike suggested that too.

+ *  pool works.
+ */
+class SkPoolDiscardableMemory;
+class SkDiscardableMemoryPool : public SkDiscardableMemory::Factory {
+public:
+    // Without mutex, will be not be thread safe.
+    SkDiscardableMemoryPool(size_t budget, SkBaseMutex* mutex = NULL);
+    ~SkDiscardableMemoryPool();
+    SkDiscardableMemory* create(size_t bytes);
+    size_t getRAMUsed();
+    void setRAMBudget(size_t budget);
+    void dumpPool();
+    friend class SkPoolDiscardableMemory;
+    #if LAZY_CACHE_STATS
+    int          fCacheHits;
+    int          fCacheMisses;
+    #endif  // LAZY_CACHE_STATS
+private:
+    SkBaseMutex* fMutex;
+    size_t       fBudget;
+    size_t       fUsed;
+    SkTInternalLList<SkPoolDiscardableMemory> fList;
+    void dumpDownTo(size_t budget);
+    void free(SkPoolDiscardableMemory* dm);

[scroggo, 2013/12/03 23:00:01]

I think I follow all your functions, but comments would be nice.

One slightly confusing aspect that would be nice to document here in the header
is that free is called by SkPoolDiscardableMemory's destructor to free the
SkPoolDiscardableMemory's memory.

Also, the fact that these functions are only called by SkPoolDiscardableMemory.

[hal.canary, 2013/12/04 00:01:36]

Done.

+    bool lock(SkPoolDiscardableMemory* dm);
+    void unlock(SkPoolDiscardableMemory* dm);
+    typedef SkNoncopyable INHERITED;

[scroggo, 2013/12/03 23:00:01]

SkDiscardableMemory::Factory should be set to INHERITED.

[hal.canary, 2013/12/04 00:01:36]

Done.

+};
+
+/**
+ *  Used for calling getRAMUsed(), setRAMBudget() or dumpPool(), or to

[scroggo, 2013/12/03 23:00:01]

I would say that this returns (and possibly creates) a threadsafe global
SkDiscardableMemoryPool.

[hal.canary, 2013/12/04 00:01:36]

Done.

+ *  act as a SkDiscardableMemory::Factory
+ */
+SkDiscardableMemoryPool* SkGetGlobalDiscardableMemoryPool();
+
+/*
+ *  Motivation: by combining SkCreateGlobalPoolDiscardableMemory and

[scroggo, 2013/12/03 23:00:01]

Motivation for what? Is this the motivation for the whole CL? If so, I think
this belongs in the change description, rather than here.

[hal.canary, 2013/12/04 00:01:36]

Done.  Those comments were meant for myself; I meant to move that text into the
CL.

+ *  SkDiscardablePixelRef, we can replace SkImageRef_GlobalPool,
+ *  currently in use by Android.  We can also properly test
+ *  SkDiscardablePixelRef as a SkLazyPixelRef within the Skia tools.

[scroggo, 2013/12/03 23:00:01]

SkLazyPixelRef is removed, correct?

[hal.canary, 2013/12/04 00:01:36]

yes.  I meant to say that I wanted to use  DiscardablePixelRefs wherever
LazyPixelRefs are used inside the skia tools and tests.

+ */
+
+/**
+ *  The same signature as SkDiscardableMemory::Create, but there are
+ *  situations where we want to choose types of DiscardableMemory -
+ *  see Android's BitmapFactory for an example.
+ *
+ *  Only returns NULL if malloc fails.
+ **/
+inline SkDiscardableMemory* SkCreateGlobalPoolDiscardableMemory(size_t bytes) {
+    return SkGetGlobalDiscardableMemoryPool()->create(bytes);
+}
+
+#if !defined(SK_DEFAULT_GLOBAL_DISCARDABLE_MEMORY_POOL_SIZE)
+#define SK_DEFAULT_GLOBAL_DISCARDABLE_MEMORY_POOL_SIZE (128 * 1024 * 1024)
+#endif
+
+#endif  // SkDiscardableMemoryPool_DEFINED