Bug 67899 – Make sure to GC decoded images that are only used with WebGL

• Status: RESOLVED
• Resolution: FIXED
• Component: WebGL

[John Bauman]

Make sure to GC decoded images that are only used with WebGL

[John Bauman]

Created attachment 106994 [details]
Patch

[John Bauman]

See http://code.google.com/p/chromium/issues/detail?id=94444 for why this is useful. With the patch, the amount of memory used only barely goes up with time, instead of causing the renderer to run out of address space

I'm not sure why adding didDraw is necessary for the image to be collected. There are a bunch of ways to use an image that may not cause that method to be called, so it might be nice to have some other way to GC them.

[Kenneth Russell]

Nice catch.

Could you instrument CachedResource::didAccessDecodedData and see which code path is taken for the images in question? It would be good to know exactly what interaction is going on with the memory cache -- in particular, whether the image is in the cache (presumably it is) and whether it's considered to be live.

[James Robinson]

didDraw() kicks the memory cache and (potentially) triggers eviction, so it's possible on your test case that there's nothing else going on, there's nothing to trigger MemoryCache::prune() other than the didDraw call from WebGL.

[John Bauman]

Yeah, looks like MemoryCache::prune is never being called, because the app only loads images and does WebGL rendering, neither of which cause that. It might be nice to also do pruning when images are loaded in case some other path forgets to do didDraw, though this patch is still valuable in updating the LRU list.

[James Robinson]

Comment on attachment 106994 [details]
Patch

R=me

[John Bauman]

Comment on attachment 106994 [details]
Patch

Oops, forgot to set that it needs commit queue. James?

[WebKit Review Bot]

Comment on attachment 106994 [details]
Patch

Rejecting attachment 106994 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2

Last 500 characters of output:
 name: third_party/ots
    url: From('chromium_deps', 'src/third_party/ots')
    should_process: True
    requirements: set(['third_party', 'chromium_deps', './'])
  
    name: third_party/snappy/src
    url: From('chromium_deps', 'src/third_party/snappy/src')
    should_process: True
    requirements: set(['third_party', 'chromium_deps', './'])

Died at Tools/Scripts/update-webkit-chromium line 80.
No such file or directory at /mnt/git/webkit-commit-queue/Tools/Scripts/webkitdirs.pm line 1929.

Full output: http://queues.webkit.org/results/9661518

[Kenneth Russell]

Comment on attachment 106994 [details]
Patch

Looks like a flake. Let's try that again.

[WebKit Review Bot]

Comment on attachment 106994 [details]
Patch

Clearing flags on attachment: 106994

Committed r95150: <http://trac.webkit.org/changeset/95150>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.