Improvements/additions to SkImageCache/SkLazyPixelRef.

Improvements/additions to SkImageCache/SkLazyPixelRef.

SkPurgeableImageCache:
New image cache that uses virtual memory to store the pixels. Combines
features of SkAshmemImageCache (which has been removed) with SkPurgeableMemoryBlock, which has android and Mac versions.

SkImageCache:
Modified the API. pinCache now returns a status out parameter which
states whether the pinned memory retained the old data. This allows
allocAndPinCache to only be used for allocations.
Add a new debug only interface to purge unpinned data.
Updates to documentation, clarifying behavior.
Changed CachedStatus to MemoryStatus

SkLruImageCache:
Implement the new function purgeAllUnpinnedCaches and change implementation
of pinCache for the new behavior.

SkLazyPixelRef:
Rewrite onLockPixels to account for the new behavior of pinCache.

BitmapFactoryTest:
Test the new SkPurgeableImageCache.
Write tests which directly test the SkImageCaches.
Create a larger bitmap, since some of the SkImageCaches are designed
to handle large bitmaps.

bench_ and render_pictures:
Consolidate lazy_decode_bitmap into one function.
Allow using a flag to specify using the purgeable image cache.
Clean up some #includes.

Committed: https://code.google.com/p/skia/source/detail?r=8207

[reed1, 2013-03-07 14:10:22]

Can we encapsulate this "purgable allocation block" OS feature in some Skia
abstraction, the way we do SkMutex or SkFileIterator? If we can, then
1. we can use it in other, non-image, places perhaps
2. We might not need two different classes (SkAshmemImageCache and
SkMacImageCache)

https://codereview.chromium.org/12433020/diff/2001/include/lazy/SkImageCache.h
File include/lazy/SkImageCache.h (right):

https://codereview.chromium.org/12433020/diff/2001/include/lazy/SkImageCache....
include/lazy/SkImageCache.h:36: enum PurgeStatus {
This is only used for pin(), so perhaps a different name?

enum PinStatus {
    kUninitialized_PinStatus,
    kRetained_PinStatus
};

Not sure that is best, but PurgeStatus sounds like the state of a purge, which
is in fact only one state.

https://codereview.chromium.org/12433020/diff/2001/include/lazy/SkImageCache....
include/lazy/SkImageCache.h:90: enum PinStatus {
MemoryStatus?

kPinned
kUnpinned
kFreed

[scroggo, 2013-03-07 18:07:19]

> Can we encapsulate this "purgable allocation block" OS
> feature in some Skia abstraction, the way we do SkMutex or
> SkFileIterator? If we can, then
> 1. we can use it in other, non-image, places perhaps
> 2. We might not need two different classes
> SkAshmemImageCache and SkMacImageCache)

Yes. I'll work that into the next patch.

https://codereview.chromium.org/12433020/diff/2001/include/lazy/SkImageCache.h
File include/lazy/SkImageCache.h (right):

https://codereview.chromium.org/12433020/diff/2001/include/lazy/SkImageCache....
include/lazy/SkImageCache.h:36: enum PurgeStatus {
On 2013/03/07 14:10:22, reed1 wrote:
> This is only used for pin(), so perhaps a different name?
> 
> enum PinStatus {
>     kUninitialized_PinStatus,
>     kRetained_PinStatus
> };
> 
> Not sure that is best, but PurgeStatus sounds like the state of a purge, which
> is in fact only one state.

Agreed. I'll think about a better name.

[scroggo, 2013-03-13 20:24:53]

> Can we encapsulate this "purgable allocation block" OS
> feature in some Skia abstraction, the way we do SkMutex or
> SkFileIterator? If we can, then
> 1. we can use it in other, non-image, places perhaps
> 2. We might not need two different classes
> SkAshmemImageCache and SkMacImageCache)

Done in the latest patch.

https://codereview.chromium.org/12433020/diff/2001/include/lazy/SkImageCache.h
File include/lazy/SkImageCache.h (right):

https://codereview.chromium.org/12433020/diff/2001/include/lazy/SkImageCache....
include/lazy/SkImageCache.h:36: enum PurgeStatus {
On 2013/03/07 18:07:19, scroggo wrote:
> On 2013/03/07 14:10:22, reed1 wrote:
> > This is only used for pin(), so perhaps a different name?
> > 
> > enum PinStatus {
> >     kUninitialized_PinStatus,
> >     kRetained_PinStatus
> > };
> > 
> > Not sure that is best, but PurgeStatus sounds like the state of a purge,
which
> > is in fact only one state.
> 
> Agreed. I'll think about a better name.

I ended up going with DataStatus, since it is the status of the data (i.e.
whether or not it contains the old data).

https://codereview.chromium.org/12433020/diff/2001/include/lazy/SkImageCache....
include/lazy/SkImageCache.h:90: enum PinStatus {
On 2013/03/07 14:10:22, reed1 wrote:
> MemoryStatus?
> 
> kPinned
> kUnpinned
> kFreed

Done.

[reed1, 2013-03-13 20:41:34]

starting to get very yummy. perhaps we land SkPurgeableMemoryBlock as its own
CL?

https://codereview.chromium.org/12433020/diff/42002/src/lazy/SkPurgeableMemor...
File src/lazy/SkPurgeableMemoryBlock.h (right):

https://codereview.chromium.org/12433020/diff/42002/src/lazy/SkPurgeableMemor...
src/lazy/SkPurgeableMemoryBlock.h:22: *  Create a new purgeable memory block
with size >= size. Returns NULL if not supported on this
size >= size is a funny statement. How about

Create a new purgeable memory block of 'size' bytes.

Do we need to say that it might be larger under the hood?

https://codereview.chromium.org/12433020/diff/42002/src/lazy/SkPurgeableMemor...
src/lazy/SkPurgeableMemoryBlock.h:25: *  @return A new block, or NULL on
failure. On success, the memory will already be pinned.
Why should it already be pinned?

https://codereview.chromium.org/12433020/diff/42002/src/lazy/SkPurgeableMemor...
src/lazy/SkPurgeableMemoryBlock.h:79: *  Pin the memory for use. Must not be
called while already pinned.
Why is this api different from the decoder, in that here there are two separate
calls: pin() and data(), instead of one: void* pin(PinResult*)

Not critical, but that sort of api makes it a little clearer that the void* (1)
may change between pin calls, and (2) it is only valid during a pin.

[scroggo, 2013-03-13 22:06:42]

> starting to get very yummy. perhaps we land SkPurgeableMemoryBlock as its own
> CL?

I made the changes here, but I'll post another CL with just
SkPurgeableMemoryBlock as well.

One potentially interesting side effect of changing pin() is that now we can
reuse the same SkPurgeableMemoryBlock if pin fails. SkPurgeableImageCache does
not currently take advantage of that, but it could.

https://codereview.chromium.org/12433020/diff/42002/src/lazy/SkPurgeableMemor...
File src/lazy/SkPurgeableMemoryBlock.h (right):

https://codereview.chromium.org/12433020/diff/42002/src/lazy/SkPurgeableMemor...
src/lazy/SkPurgeableMemoryBlock.h:22: *  Create a new purgeable memory block
with size >= size. Returns NULL if not supported on this
On 2013/03/13 20:41:34, reed1 wrote:
> size >= size is a funny statement. How about
> 
> Create a new purgeable memory block of 'size' bytes.
> 
> Do we need to say that it might be larger under the hood?

Done.

https://codereview.chromium.org/12433020/diff/42002/src/lazy/SkPurgeableMemor...
src/lazy/SkPurgeableMemoryBlock.h:25: *  @return A new block, or NULL on
failure. On success, the memory will already be pinned.
On 2013/03/13 20:41:34, reed1 wrote:
> Why should it already be pinned?

Currently, the only client (allocAndPinCache) wants the memory to be pinned when
it asks for it. I uploaded a patch that changes this behavior so we can decide
if we like that better.

https://codereview.chromium.org/12433020/diff/42002/src/lazy/SkPurgeableMemor...
src/lazy/SkPurgeableMemoryBlock.h:79: *  Pin the memory for use. Must not be
called while already pinned.
On 2013/03/13 20:41:34, reed1 wrote:
> Why is this api different from the decoder, in that here there are two
separate
> calls: pin() and data(), instead of one: void* pin(PinResult*)
> 
> Not critical, but that sort of api makes it a little clearer that the void*
(1)
> may change between pin calls, and (2) it is only valid during a pin.

In the implementation from this patch, it starts off pinned, and there needs to
be a way to access the data without calling pin. The latest patch changes it so
it does not start off pinned, so I can use that API.

[scroggo, 2013-03-14 19:51:04]

On 2013/03/13 22:06:42, scroggo wrote:
> > starting to get very yummy. perhaps we land SkPurgeableMemoryBlock as its
own
> > CL?
> 
> I made the changes here, but I'll post another CL with just
> SkPurgeableMemoryBlock as well.

The other CL is at https://codereview.chromium.org/12645006/. Once we are happy
with that one and check it in I will update this one to remove the duplication.

[scroggo, 2013-03-15 18:52:58]

Latest patch ready for review.

[reed1, 2013-03-18 21:25:17]

https://codereview.chromium.org/12433020/diff/69001/include/lazy/SkPurgeableI...
File include/lazy/SkPurgeableImageCache.h (right):

https://codereview.chromium.org/12433020/diff/69001/include/lazy/SkPurgeableI...
include/lazy/SkPurgeableImageCache.h:28: static SkImageCache*
GetPurgeableImageCache();
Why isn't this just called Create()?

[reed1, 2013-03-18 21:28:27]

lgtm if we rename the factory to something like Create, and remove the dox that
allude to a singleton (unless that is an important feature of the class)

[scroggo, 2013-03-18 21:37:16]

> lgtm if we rename the factory to something like Create, and remove the dox
> that allude to a singleton (unless that is an important feature of the class)

Done.

https://codereview.chromium.org/12433020/diff/69001/include/lazy/SkPurgeableI...
File include/lazy/SkPurgeableImageCache.h (right):

https://codereview.chromium.org/12433020/diff/69001/include/lazy/SkPurgeableI...
include/lazy/SkPurgeableImageCache.h:28: static SkImageCache*
GetPurgeableImageCache();
On 2013/03/18 21:25:17, reed1 wrote:
> Why isn't this just called Create()?

Done.

[scroggo, 2013-03-18 21:38:37]

Committed patchset #13 manually as r8207 (presubmit successful).