67899 2011-09-10 18:01:38 -0700 Make sure to GC decoded images that are only used with WebGL 2011-09-14 19:11:46 -0700 1 1 1 Unclassified WebKit WebGL 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 jbauman jbauman jamesr kbr webkit.review.bot oldest_to_newest 465361 0 jbauman 2011-09-10 18:01:38 -0700 Make sure to GC decoded images that are only used with WebGL 465363 1 106994 jbauman 2011-09-10 18:03:54 -0700 Created attachment 106994 Patch 465365 2 jbauman 2011-09-10 18:09:07 -0700 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. 465723 3 kbr 2011-09-12 12:10:07 -0700 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. 465993 4 jamesr 2011-09-12 17:43:21 -0700 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. 466109 5 jbauman 2011-09-12 21:29:53 -0700 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. 466360 6 106994 jamesr 2011-09-13 10:09:15 -0700 Comment on attachment 106994 Patch R=me 467018 7 106994 jbauman 2011-09-14 07:18:58 -0700 Comment on attachment 106994 Patch Oops, forgot to set that it needs commit queue. James? 467098 8 106994 webkit.review.bot 2011-09-14 10:08:34 -0700 Comment on attachment 106994 Patch Rejecting attachment 106994 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 467160 9 106994 kbr 2011-09-14 11:15:23 -0700 Comment on attachment 106994 Patch Looks like a flake. Let's try that again. 467540 10 106994 webkit.review.bot 2011-09-14 19:11:41 -0700 Comment on attachment 106994 Patch Clearing flags on attachment: 106994 Committed r95150: <http://trac.webkit.org/changeset/95150> 467541 11 webkit.review.bot 2011-09-14 19:11:46 -0700 All reviewed patches have been landed. Closing bug. 106994 2011-09-10 18:03:54 -0700 2011-09-14 19:11:41 -0700 Patch bug-67899-20110910180353.patch text/plain 1684 jbauman U3VidmVyc2lvbiBSZXZpc2lvbjogOTQ4MjEKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0No YW5nZUxvZyBiL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwppbmRleCBiY2ZhNTlkNWM5YWM3MmI5 YTAwYzMxMTMyMTE0M2I5YjE1MzViMTE0Li40NzBiZDNiNzJjMmQ5YmRkMjk5ZmQ5NDBlMDRlZGMx OTUzYjI2MTdlIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKKysrIGIvU291 cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTYgQEAKKzIwMTEtMDktMTAgIEpvaG4g QmF1bWFuICA8amJhdW1hbkBjaHJvbWl1bS5vcmc+CisKKyAgICAgICAgTWFrZSBzdXJlIHRvIEdD IGRlY29kZWQgaW1hZ2VzIHRoYXQgYXJlIG9ubHkgdXNlZCB3aXRoIFdlYkdMCisgICAgICAgIGh0 dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD02Nzg5OQorCisgICAgICAgIFJl dmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIFRlbGwgQ2FjaGVkSW1hZ2UgdGhh dCB0aGUgV2ViR0wgaW1hZ2Ugd2FzIGRyYXduIHNvIFdlYkdMLW9ubHkKKyAgICAgICAgY29udGVu dCB3aWxsIGhhdmUgZGVjb2RlZCBpbWFnZXMgY29sbGVjdGVkLgorCisgICAgICAgICogcGxhdGZv cm0vZ3JhcGhpY3MvR3JhcGhpY3NDb250ZXh0M0QuY3BwOgorICAgICAgICAoV2ViQ29yZTo6R3Jh cGhpY3NDb250ZXh0M0Q6OmV4dHJhY3RJbWFnZURhdGEpOgorCiAyMDExLTA5LTA4ICBEYXZpZCBM ZXZpbiAgPGxldmluQGNocm9taXVtLm9yZz4KIAogICAgICAgICBbY2hyb21pdW1dIEtVUkw6OmNv cHkgZG9lc24ndCBwcm9kdWNlIHNvbWV0aGluZyB1c2FibGUgb24gYW5vdGhlciB0aHJlYWQuCmRp ZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9HcmFwaGljc0NvbnRl eHQzRC5jcHAgYi9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9HcmFwaGljc0NvbnRl eHQzRC5jcHAKaW5kZXggY2I2ZDQ4YzNkZDkzYTM2ZDc5NzMxOGJhZGVjZDFkYTM3YzYwMDQzMi4u ZWMyMTg2ZGNlNjE5MTY2ZmJmY2ZmOWE2YjIzZGMxMjUwYjRkMTNkZCAxMDA2NDQKLS0tIGEvU291 cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3MvR3JhcGhpY3NDb250ZXh0M0QuY3BwCisrKyBi L1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL0dyYXBoaWNzQ29udGV4dDNELmNwcApA QCAtMzYsNiArMzYsNyBAQAogI2luY2x1ZGUgIkV4dGVuc2lvbnMzRC5oIgogI2luY2x1ZGUgIklt YWdlLmgiCiAjaW5jbHVkZSAiSW1hZ2VEYXRhLmgiCisjaW5jbHVkZSAiSW1hZ2VPYnNlcnZlci5o IgogCiAjaW5jbHVkZSA8d3RmL093bkFycmF5UHRyLmg+CiAjaW5jbHVkZSA8d3RmL1Bhc3NPd25B cnJheVB0ci5oPgpAQCAtMTg5LDYgKzE5MCw4IEBAIGJvb2wgR3JhcGhpY3NDb250ZXh0M0Q6OmV4 dHJhY3RJbWFnZURhdGEoSW1hZ2UqIGltYWdlLAogICAgICAgICAgICAgICAgICAgICAgICBjb21w b25lbnRzUGVyUGl4ZWwgKiBieXRlc1BlckNvbXBvbmVudCwKICAgICAgICAgICAgICAgICAgICAg ICAgdW5wYWNrQWxpZ25tZW50KTsKICAgICB9CisgICAgaWYgKEltYWdlT2JzZXJ2ZXIgKm9ic2Vy dmVyID0gaW1hZ2UtPmltYWdlT2JzZXJ2ZXIoKSkKKyAgICAgICAgb2JzZXJ2ZXItPmRpZERyYXco aW1hZ2UpOwogICAgIHJldHVybiB0cnVlOwogfQogCg==