In https://bugzilla.mozilla.org/show_bug.cgi?id=963917#c128 , we can see a lot of blob-url object in memory report. We should only preserve some blob-url object, not for all image. John has a wip patch to reduce the number of blob-url object.

I'm concerned about the impact of John's change on scrolling speed, and think this will need careful testing before landing. Also, I'd like to get some gecko developers involved in this bug to find out why each blob url is consuming multiple kilobytes of memory. I thought that if a blob url refered to a file-backed blob it was little more than an entry in a hash table of some sort. Why do 2000 blob urls take almost 10mb of memory. Why do 187 blob urls take 650kb of memory? Can we reduce the memory usage in gecko rather than reduce url usage in Gallery? Also note that bug 967089 is filed to remove the visibility monitor code from Gallery. Apparently if we convert thumbnails to use real img elements instead of CSS backgroundImage, gecko will now do the visiblility monitoring for us and swap images in and out of memory as needed. I would like to try doing that, because I think it will remove a layer of overhead and probably make scrolling smoother. Note, however, that this patch to remove blob urls depends on the visibility monitor, so we can't do both. Furthermore, Jerry has pointed out that at some point we end up having too many DOM elements if we always create one for each thumbnail. If we're serious about not wanting memory use to be linear with the number of photos, we need to come up with a different scrolling solution than the visibility monitor so that we can actually create new thumbnail elements as needed, rather than just swapping their blob urls in and out. To summarize, my reasons not to land John's patch are: 1) It might degrade scrolling performance and take longer to swap images in. That will need testing 2) It will make it impossible to improve scrolling by removing the visibility monitor. 3) It is not a general-purpose solution: we still have one DOM element for each thumbnail. I would like someone familiar with Gecko's implementation of blob urls to look at the memory figures we're seeing and see if they make sense. Ben or Kyle, are either of you able to look into this? Does it seem reasonable that each blob url consumes 3 to 5 kb of memory? Of course if we've got an urgent need for memory savings for our low-memory devices, I'm willing to consider landing this as a short-term fix, as long as we can get enough QA on it.

(In reply to David Flanagan [:djf] from comment #1) > 1) It might degrade scrolling performance and take longer to swap images in. > That will need testing That may be true. I had the same concern while creating the WIP patch and already told Jerry. We feel it may be a trade-off on memory and smooth.

This is my test image(512x512 png file).

(In reply to Jerry Shih[:jerry] from comment #3) > Created attachment 8374609 [details] > img2.png > > This is my test image(512x512 png file). The test image at: https://bugzilla.mozilla.org/show_bug.cgi?id=963917#c140

This is a simple test. --- var imageFile; var url; function load() { var pics = navigator.getDeviceStorage('pictures'); var req = pics.get("/sdcard/img.png"); req.onsuccess = function(e) { imageFile = req.result; }; } function create() { url = URL.createObjectURL(imageFile); } function show() { document.getElementById('holder').src = url; } function hide() { document.getElementById('holder').src = ''; } function revoke() { URL.revokeObjectURL(url); url = null; } --- My test step: 1. call load() no blob object occur 2. call create() no blob object occur 3. call show() 1 blob-urls occur 4. call hide() 1 blob-urls still occur 5. call revoke() no blob object occur This is our gallery implement: --- apps/gallery/js/thumbnail_item.js line:30 this.htmlNode.dataset.backgroundImage = 'url("' + url + '")'; --- This test shows that the "thumbnail.dataset.backgroundImage" is hold "a buffer", not a file blob.

Jerry and I looked at this briefly, the behavior in comment 5 is strange, we should see a blob-url after calling create().

The test app(comment 5) comes from John. It will load a file "/sdcard/test.jpg" in device. So we should put a file at this location before using this test app.

When we use about:memory to dump the blob-url, it will dump a blob size at: http://dxr.mozilla.org/mozilla-central/source/content/base/src/nsHostObjectProtocolHandler.cpp#177 In this test, with gdb, the blob is file-base blob. The GetSize() is called at: http://dxr.mozilla.org/mozilla-central/source/content/base/src/nsDOMFile.cpp#178 So, maybe gallery can ignore this blob-url size.

Fix the incorrect blob report in comment 6.

After we fix the bug 972799, we can run the about:memory tool again to see the memory usage.

With bug 972799 patch, the memory report can handle "memory-base" and "file-base" blob object now. There are 67 jpeg images(1600*1200 up) in gallery. Before bug 963917 fixed, we can see there are a lot of memory-base blobs in memory report at 1st launch. 2.76 MB (100.0%) -- memory-blob-urls └──2.76 MB (100.0%) -- owner(app://gallery.gaiamobile.org/index.html) .......... ├──0.03 MB (01.05%) ── blob:5050adb1-1260-45b3-8d60-d7c10bdc87d5 └──0.03 MB (01.00%) ── blob:11be602f-f8db-4104-be58-8d73daad4c8b After bug 963917 fixed, these blobs become file-base blob. 67 (100.0%) -- file-blob-urls └──67 (100.0%) -- owner(app://gallery.gaiamobile.org/index.html) .......... ├───1 (01.49%) ── blob:fdbff45d-e363-489b-9cca-9eb6dea26e84 └───1 (01.49%) ── blob:ffd701ad-4366-416e-9ae0-23532ca8f921 If the file-base blob is just consuming a little memory, maybe we can ignore the file-blob-urls count. Kyle, is it true for my description?

Yes, so you can see correctly that we have 67 blob URLs for file based blobs. They should only take up a few bytes each (enough for the hashtable entry), so we can ignore them. Memory based blobs are the ones we need to worry about, and if there aren't any of those, we can close this bug.

According to the comment 11 and comment 12, the gallery uses file-base blob now. We can ignore the file-base blob usage.

This is nit-picking, but I'm going to call it INVALID instead of WONTFIX because the tools were wrong and there was never a real problem here.