Bug 112374 – [chromium] Scale up cache limit in ImageDecodingStore by device scale factor

• Status: RESOLVED
• Resolution: WONTFIX
• Component: Images

[Hin-Chung Lam]

This is a quick solution to solve the problem of cache exhaustion on hidpi devices. These devices are using an explicit up-sampling to provide better quality. Scaled up images are saved in memory and quickly exhaust the cache limit. Bump up the cache limit by device scale factory (or maybe sqaure of it) can help.

[Hin-Chung Lam]

Created attachment 193692 [details]
Patch

[Hin-Chung Lam]

The related bug in crbug.com is here: https://code.google.com/p/chromium/issues/detail?id=189390

Please let me know if this is the right approach. Or we should have an additional WebKit API to do this.

[James Robinson]

Comment on attachment 193692 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=193692&action=review

Seems like a bit of a hack, but I don't have a better idea off the top of my head.

> Source/WebCore/platform/graphics/chromium/ImageDecodingStore.cpp:38
> +static const size_t cDefaultCacheLimitInBytes = 32768 * 1024;

we don't use prefixes like this in WebKit

[Hin-Chung Lam]

I agree this is a hack.

My plan to fix this is:
1. Find a better way to expose this and set it relative to device scale factor.
2. Better prioritization of decoding tasks in tiling so this hack might not be necessary.

[Hin-Chung Lam]

Created attachment 193699 [details]
Patch

[Alexandre Elias]

I'm afraid this will cause OOMs on Android phones.  Last I checked this cache value was sent down from the embedder, could you apply the multiplication in platform-specific code on the Chromium side?

[Hin-Chung Lam]

I'll look to see if there's a better way.

[Nat Duca]

Why would this oom android phones? They have deferreds, no, so this stuff isn't live?

[Hin-Chung Lam]

If ashmem is exhausted then heap memory is used, not sure if this is the case on Android.

[Alexandre Elias]

I'm not sure whether it's really a problem or not.  Min, is it?

[Min Qin]

Yes, heap memory will be used when we can no longer allocate any ashmem. Since there is a file descriptor limit of 128, we could hit that limit and start to allocate in heap.

(In reply to comment #10)
> I'm not sure whether it's really a problem or not.  Min, is it?

[Nat Duca]

As an aside, then, can we maybe do a cleanup of the discardable logic in some way so its either discardable or nothing? Or something a little easier to reason about? I think its really hard to reason about the memory footprint of something that mixes discardables with mallocs depending on the number of outstanding fd's.

[Min Qin]

There could be other reasons we don't favor discardable memory even it is enabled. For example, allocation size. If the allocation size is small, it is better to allocate in heap instead of discardable memory. Currently this is not enforced, but i am planning to add that in the future.

(In reply to comment #12)
> As an aside, then, can we maybe do a cleanup of the discardable logic in some way so its either discardable or nothing? Or something a little easier to reason about? I think its really hard to reason about the memory footprint of something that mixes discardables with mallocs depending on the number of outstanding fd's.