The memory reports in bug 1028260 show that the homescreen memory usage went up significantly between 1.4 and 2.0. 7,397,376 B (100.0%) -- explicit ├──5,577,888 B (75.40%) ── heap-unclassified

(Comment 0 is a diff between 1.4 and 2.0.)

erahm is going to run DMD.

I'll drive this for now.

b2g-info details: NAME PID PPID CPU(s) NICE USS PSS RSS VSIZE OOM_ADJ USER Homescreen 1.3 1075 937 15.2 18 10.6 12.6 27.3 67.9 8 u0_a1075 Homescreen 1.4 955 835 4.5 1 12.3 17.5 28.0 79.8 2 u0_a955 Homescreen 2.0 1047 842 5.9 1 19.4 26.2 43.2 113.2 2 u0_a1047 Homescreen 2.1 1051 834 6.3 18 19.6 26.4 43.5 107.4 8 u0_a1051

Adding Systems FE since this seems related to the vertical homescreen per the dependency. I'll look into seeing if I can get someone on the Systems FE team to work with Kyle to resolve this.

See: https://bugzilla.mozilla.org/show_bug.cgi?id=1028260#c6 Sounds like we should profile this again to see if there really is an issue.

Still working on DMD, here are average stats rebooting and measuring 4X on 1.4, 2.0, 2.1. USS PSS RSS VSIZE Homescreen 1.4 11.45 16.525 27.05 78.375 Homescreen 2.0 18.35 26 42.15 112.85 Homescreen 2.1 15.925 22.825 38.225 99.025 And deltas: Deltas USS RSS 1.4 -> 2.0 6.9 15.1 2.0 -> 2.1 -2.425 -3.925 1.4 -> 2.1 4.475 11.175 So we can see that 2.1 has some improvements that might make sense to port to 2.0.

I'd like to see a vertical homescreen test using the features requested in bug 1022818. There's a testing patch to try and see if that puts a dent in the memory spike.

(In reply to Jet Villegas (:jet) from comment #8) > I'd like to see a vertical homescreen test using the features requested in > bug 1022818. There's a testing patch to try and see if that puts a dent in > the memory spike. We are currently not using any smooth scrolling features in 2.0, but we would like to consider them for 2.1. I don't think that should impact 2.0, so can we handle that in another bug?

This is captured using following STR: 1) Open gallery and zoom in/out a pic 2) Minimize gallery and try to open Camera 3) Do camera recording and stop it 4) Minimize camera app. Observation: Peak memory memory usage of homescreen has reached >36MB (USS memory of homescreen process) at some point.

After talking to Kyle, we need the DMD fix to do more investigation here, so adding a dependency.

Investigation into the DMD on flame issue is taking longer than expected -- I'm going to try DMD on hamachi and see if that works. If so I should have some detailed reports soon.

heap-unclassified on a 2.0 buri device is quite low (1.78 MB), DMD doesn't show anything of interest (the largest block of unclassified memory is 60KB). My best guess is that the difference is coming from the graphics layer (different resolution, gpu, hardware acceleration etc).

With DMD now working I was able to get more insight into the issue. The majority of heap-unclassified is indeed graphics memory. The top 6 entries are all graphics related, accounting for 2MB+ of data.

There are couple of issues with vertical homescreen. 1) total kgsl memory allocation is 11MB in v2.0 and it is only 3MB in v1.3. I have seen this after rebooting device. So we are seeing 8MB memory growth in kgsl allocation between v2.0 and v1.3 2) I checked total ion allocation from system heap after reboot. I found v1.3 is showing 8MB allocation and v2.0 is showing 14MB. So we are loosing 6MB here 3) I measured total ion allocation from system heap after panning homescreen. I found that v1.3 is not showing any significant difference of memory usage during scrolling. But in v2.0, total ion allocation is increasing from 14MB to 21MB during scrolling. So we are loosing 7MB here. 4) b2g-procrank shows that homescreen is using 19MB(USS) after rebooting in v2.0 and it is showing only 12MB (USS) in v1.3 after rebooting. Please note that USS memory usage is not counted in ion allocation. We are loosing 7MB here just after rebooting device. 5) b2g-procrank shows that homescreen is using 21MB(USS) after panning in v2.0 and it is showing only 12MB (USS) in v1.3 after panning. This means that vertical homescreen allocates more memory if we pan it. We are loosing 3MB memory during scrolling vertical homescreen. You can also see that different memory usage of vertical homescrren in b2g-procrank command at #comment 10. We are seeing peak memory usage at 36MB in #comment 10. This is forcing all background apps to be killed very frequently. Is there any way to switch back to horizontal homescreen app in v2.0 ? I want to see memory usage difference between vertical and horizontal homescreen app. This can tell us why both total ion allocation and kgsl allocation is so high just after rebooting in v2.0 compared to v1.3. I am also not sure whether gecko DMD memory report is counting both ion allocation and kgsl allocation or not. These are mainly coming from graphics layer tree. Could you please confirm this ?

In summary, we are seeing total memory growth as 8MB+6MB+7MB+7MB+3MB = 31MB for vertical homescreen when we scroll it (steps 1,2,3,4,5 in #comment 15). My guess is that gecko memory report does not count all of these memory. We are also seeing homescreen USS reaches 36MB in b2g-procrank in #comment 10. @mvines, Should we disable this vertical homescreen till mozilla fix these issues ? Please suggest. @Eric: Can you please give us a simple patch which will enable horizontal homescreen for us again ? We just want to see total ion and kgsl memory usage for horizontal homescreen app in v2.0.

Kevin is the right person to ask about testing the old homescreen on 2.0.

You need to change 'verticalhome' to 'homescreen' here: https://github.com/mozilla-b2g/gaia/blob/master/build/settings.js#L169

(In reply to Tapas Kumar Kundu from comment #16) > In summary, we are seeing total memory growth as 8MB+6MB+7MB+7MB+3MB = 31MB > for vertical homescreen when we scroll it (steps 1,2,3,4,5 in #comment 15). > My guess is that gecko memory report does not count all of these memory. > We are also seeing homescreen USS reaches 36MB in b2g-procrank in #comment > 10. > > @mvines, Should we disable this vertical homescreen till mozilla fix these > issues ? Please suggest. Note - post enabling of the vertical homescreen, we stopped maintaining the old homescreen for 2.0. That means there's no guarantee on what's going to work on the old homescreen right now vs. not work.

Yup, what Fabrice said in comment 18 should work to re-enable the vertical homescreen. This would work for testing, but not a production-quality patch of course. Let me know if there's anything else I can help with.

Switching back to the old homescreen probably won't help get the product out quicker, we need to fix the new homescreen ASAP if it will remain in v2.0

(In reply to Tapas Kumar Kundu from comment #15) > Created attachment 8451285 [details] > Homescreen_memory_usage_comparison between v1.3 and v2.0 > > There are couple of issues with vertical homescreen. > > 1) total kgsl memory allocation is 11MB in v2.0 and it is only 3MB in v1.3. > I have seen this after rebooting device. > So we are seeing 8MB memory growth in kgsl allocation between v2.0 and v1.3 > For horizontal homescreen , we don't see additional 8MB kgsl memory usage . i.e it is becoming same as v1.3 > 2) I checked total ion allocation from system heap after reboot. I found > v1.3 is showing 8MB allocation and v2.0 is showing 14MB. So we are loosing > 6MB here > This is also present in horizontal homescreen. It seems like we are allocating more ion memory in v2.0. This is coming from graphics layer tree. > 3) I measured total ion allocation from system heap after panning > homescreen. > I found that v1.3 is not showing any significant difference of memory usage > during scrolling. But in v2.0, total ion allocation is increasing from 14MB > to 21MB during scrolling. So we are loosing 7MB here. > This does not happen with horizontal homescreen. This is specific to vertical homescreen only. > 4) b2g-procrank shows that homescreen is using 19MB(USS) after rebooting in > v2.0 and it is showing only 12MB (USS) in v1.3 after rebooting. Please note > that USS memory usage is not counted in ion allocation. We are loosing 7MB > here just after rebooting device. > With horizontal homescren, we are seeing 14MB (USS) after rebooting. So, we are still regressing 2MB here in v2.0. > 5) b2g-procrank shows that homescreen is using 21MB(USS) after panning in > v2.0 and it is showing only 12MB (USS) in v1.3 after panning. This means > that vertical homescreen allocates more memory if we pan it. > We are loosing 3MB memory during scrolling vertical homescreen. > > This does not happen with horizontal homescreen. This is specific to vertical homescreen only. So even horizontal homescreen is not solving memory regression fully but it saves some memory compared to vertical homescreen.

Some of this memory is down to gfx, I present the rough equations below: The old home-screen consisted of one main sheet, and possibly 2 more on either side kept in memory. So let's say that's 3*800*480*4 bytes, or 4.4mb or so (sheets don't take up the whole screen, so it's a bit less). We can easily discard these extra 2 sheets when they aren't necessary, so possibly 1.46mb - I don't know the code to know what it does. The new home-screen consists of a scrolling list. It's scrolling, so it'll be using tiles and have a display-port. Although tile-size is configurable, we have it at the default of 256x256, so there will be an amount of wastage on the horizontal axis (not a significant amount, 480 is very close to 512). The default display-port expansion for the y-axis is 1.8 and the x-axis is 1.5. The excess on the x-axis will get redistributed to the y-axis. So 800x480 would end up as 1440x720, which would then get redistributed to 2880x480 (see CalculateDisplayPortSize and RedistributeDisplayPortExcess in AsyncPanZoomController.cpp). This will then get rounded up to the nearest tile size, so ends up as 3072x512. This buffer takes up 6 megs (96k of which is wastage, a further 96k is tile-boundary expansion), but I've not taken into account low-precision rendering, which could push this up a bit further. It's also worth noting that scrolling around will fill up the tile cache, so it would temporarily take more memory than this (but the cache will be emptied on a memory pressure event, and I don't think the cache numbers are tuned for this large screen-size anyway). Do we know if this memory disappears when the app is sent to the background? If it isn't, there's a good few megs there we could reclaim when the home-screen isn't visible. It may also be worth considering purging tiles that don't intersect with the composition bounds and invalidating the layer in that area on memory pressure too. We could decrease the display-port size, but I think the current numbers work well. Note that this has always been known to be a memory for performance trade-off (tiles + apz end up using more memory, but you have superior performance for it - there are then further trade-offs for checkerboarding/memory). I also wonder if the default 3-column layout is hurting us memory-wise, and what happens to the icons that are outside of the display-port in terms of memory use? (to be clear, this latter comment is not just strictly gfx-related)

I hadn't read comment #22 properly before making my comment, but the numbers and behaviour tally up with what I've written.

So it seems in comment 15 we have five different issues. How do we want to handle this? We have a bug filed for KGSL, but we do not have bugs for the other issues. Should we split them into individual bugs and turn this into a meta bug?

That sounds good. The KGSL bug will deal with the graphics memory usage regression. I haven't confirmed if there's any non graphics related regression. This bug can track the KGSL + any non graphics bug (if any).

(In reply to Benoit Girard (:BenWa) from comment #26) > That sounds good. The KGSL bug will deal with the graphics memory usage > regression. I haven't confirmed if there's any non graphics related > regression. This bug can track the KGSL + any non graphics bug (if any). Thanks Benoit. Now that we have some improvements in KGSL, could that improve the USS memory situation? I am wondering if it makes sense to re-measure this.

I don't know for sure how KGSL and USS relate. I think it's worth checking.

Tapas - since we have landed the gaia patches for DPI and will-change, we are seeing a lower KGSL regression in bug 1030608. Would you be able to measure USS again (using the same methodologies in comment 15) to see if we are still showing a significant regression here?

(In reply to Kevin Grandon :kgrandon from comment #29) > Tapas - since we have landed the gaia patches for DPI and will-change, we > are seeing a lower KGSL regression in bug 1030608. Would you be able to > measure USS again (using the same methodologies in comment 15) to see if we > are still showing a significant regression here? Could you please refer me to exact patch for for DPI and will-change for FFOS 2.0 ? It will help me to test it quickly.

(In reply to Tapas Kumar Kundu from comment #30) > (In reply to Kevin Grandon :kgrandon from comment #29) > > Tapas - since we have landed the gaia patches for DPI and will-change, we > > are seeing a lower KGSL regression in bug 1030608. Would you be able to > > measure USS again (using the same methodologies in comment 15) to see if we > > are still showing a significant regression here? > > Could you please refer me to exact patch for for DPI and will-change for > FFOS 2.0 ? It will help me to test it quickly. I believe you are looking for the patches in bug 1027231 and bug 1034347. Both of which should be in the latest 2.0.

(mid-air) Don't forget bug 1026240. If they're not all on 2.0 then that is a mistake.

Looks like we have all the mentioned bug fixes. Adding back ni on Tapas as per comment 29.

(In reply to Inder from comment #33) > Looks like we have all the mentioned bug fixes. > Adding back ni on Tapas as per comment 29. There are still couple of issues with vertical homescreen. We didn't observe any significant memory usage improvement with those patches. 1) total kgsl memory allocation is 6.5MB in v2.0 and it is only 3MB in v1.3. I have seen this after rebooting device. So we are seeing 3.5MB memory growth in kgsl allocation between v2.0 and v1.3. Please note that we observed 11MB earlier in #comment 15. So New patches has saved 4.5MB kgsl memory in FFOS 2.0 . But still it is not sufficient compared to v1.3. 2) I checked total ion allocation from system heap after reboot. I found v1.3 is showing 8MB allocation and v2.0 is showing 20MB. So we are loosing 12MB ion memory here. Please note that earlier we observed in 14MB ion allocation in #comment 15 on FFOS 2.0 3) I measured total ion allocation from system heap after panning homescreen. I found that both v1.3 and v2.0 is not showing any significant increment in ion allocations during scrolling. Please compare this observation to #comment 15 step-3. 4) b2g-procrank shows that homescreen is using 26MB(USS) after rebooting in v2.0 and it is showing only 12MB (USS) in v1.3 after rebooting. Please note that USS memory usage is not counted in ion allocation. We are loosing 14MB here just after rebooting device. We are loosing more memory here compared to #comment 15. 5) b2g-procrank does not show any increment in homescreen USS for both v2.0 and v1.3 during scrolling. Please compare this observation to #comment 15 step-5 So memory regression is 3.5+12+0+14+0=29.5 MB compared to v1.3 . Please note that this data is collected on following gaia/gecko FFOS 2.0: gaia : https://github.com/mozilla-b2g/gaia/commit/f2509d29f7ee4d670d0dbc47f16e1794529aee3f gecko : https://www.codeaurora.org/cgit/quic/lf/b2g/mozilla/gecko/commit/?h=mozilla/v2.0&id=cf6d91d171f8c4b437fba56c6fd6d850292cc4ad

Thanks for measuring, I'll try to look at some memory reports. Kyle - do you have anything else immediately actionable here?

(In reply to Tapas Kumar Kundu from comment #34) > Please note that USS memory usage is not counted in ion allocation. What do you mean? (In reply to Tapas Kumar Kundu from comment #15) > Created attachment 8451285 [details] > Homescreen_memory_usage_comparison between v1.3 and v2.0 > > adb reboot && sleep 60 && adb shell b2g-procrank && adb shell cat /sys/kernel/debug/ion/heaps/system && adb shell cat /sys/class/kgsl/kgsl/page_alloc What is /sys/kernel/debug/ion/heaps/system ??? Is there a patch that I am not aware of and that you are using internally? I cannot find this file on any devices (Flame 2.0, Flame 2.1, Unagi 2.0, Unagi 2.1) that I have. I guess when you mean 12 MB (11214848 bytes), this is including all applications, such as the system app, NUWA, the preallocated app, and the homescreen? Is there a way to have a view on this patch, or to have it upstream-ed?

Looking at about-memory of the Homescreen from a Flame v2.0 reported by the B2G/tools/get-about-memory.py, I see the following result: > 22.76 MB (100.0%) -- explicit > ├───6.64 MB (29.17%) ++ images/content/raster/used > ├───4.90 MB (21.55%) ── heap-unclassified > ├───4.19 MB (18.40%) -- js-non-window > │ ├──1.23 MB (05.40%) -- runtime > │ │ ├──0.66 MB (02.92%) ── script-data > │ │ ├──0.33 MB (01.46%) ── atoms-table > │ │ └──0.23 MB (01.01%) -- (10 tiny) > │ │ ├──0.06 MB (00.27%) -- code > │ │ │ ├──0.04 MB (00.19%) ── other > │ │ │ ├──0.02 MB (00.08%) ── unused > │ │ │ ├──0.00 MB (00.00%) ── baseline > │ │ │ ├──0.00 MB (00.00%) ── ion > │ │ │ └──0.00 MB (00.00%) ── regexp Which is far from the 20 MB of Ion code.

"ion" memory is a kernel thing. It has nothing to do with Ionmonkey.

Preferences for tile sizes: pref("layers.tile-width", 256); pref("layers.tile-height", 256);

If you do test with the modified tile size (I'd try 240x240, which is not a multiple of 32, but just to see what it looks like, as well as 128x128), could we also have a memory report similar to the one discussed in comment 34?

This is on master

(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #41) > Created attachment 8456491 [details] > Master homescreen DMD log > > This is on master Seems like there is a lot of: replace_malloc /home/khuey/dev/gecko-dev/memory/replace/dmd/DMD.cpp:1245 (0xb6fa7798 libdmd.so+0x5798) os_malloc (0xb381cbf2 libgsl.so+0xdbf2) (no addr2line) (0xace438a2 libGLESv2_adreno.so+0x6f8a2) (no addr2line) rb_cmdbuffer_alloc (0xace44048 libGLESv2_adreno.so+0x70048) (no addr2line) rb_context_create (0xace4629c libGLESv2_adreno.so+0x7229c) (no addr2line) gl2_context_create (0xace233a2 libGLESv2_adreno.so+0x4f3a2) (no addr2line) eglCreateClientApiContext (0xaff6eec4 libEGL_adreno.so+0x19ec4) (no addr2line) qeglDrvAPI_eglCreateContext (0xaff6517c libEGL_adreno.so+0x1017c) (no addr2line) eglCreateContext (0xaff5bad8 libEGL_adreno.so+0x6ad8) (no addr2line) eglCreateContext /home/khuey/dev/B2G/frameworks/native/opengl/libs/EGL/eglApi.cpp:634 (0xb6e407ee libEGL.so+0xf7ee) mozilla::gl::GLLibraryEGL::fCreateContext(void*, void*, void*, int const*) /home/khuey/dev/gecko-dev/gfx/gl/GLLibraryEGL.h:180 (discriminator 3) (0xb482c95c libxul.so+0x78595c) nsRefPtr<mozilla::gl::GLContextEGL> /home/khuey/dev/gecko-dev/obj-b2g-dbg-dmd/dist/include/nsCOMPtr.h:175 (0xb482e8ea libxul.so+0x7878ea) already_AddRefed<mozilla::gl::GLContextEGL>::take() /home/khuey/dev/gecko-dev/obj-b2g-dbg-dmd/dist/include/nsCOMPtr.h:175 (0xb4834396 libxul.so+0x78d396) already_AddRefed<mozilla::gl::GLContext>::take() /home/khuey/dev/gecko-dev/obj-b2g-dbg-dmd/dist/include/nsCOMPtr.h:175 (0xb48520b6 libxul.so+0x7ab0b6) mozilla::dom::CanvasRenderingContext2D::EnsureTarget() /home/khuey/dev/gecko-dev/content/canvas/src/CanvasRenderingContext2D.cpp:944 (0xb4f777ee libxul.so+0xed07ee) mozilla::dom::HTMLImageElementOrHTMLCanvasElementOrHTMLVideoElement::IsHTMLCanvasElement() const /home/khuey/dev/gecko-dev/obj-b2g-dbg-dmd/dist/include/mozilla/dom/UnionTypes.h:2494 (0xb56bf112 libxul.so+0xed1112) drawImage /home/khuey/dev/gecko-dev/obj-b2g-dbg-dmd/dom/bindings/CanvasRenderingContext2DBinding.cpp:3295 (0xb5083daa libxul.so+0x895daa) mozilla::dom::GenericBindingMethod(JSContext*, unsigned int, JS::Value*) /home/khuey/dev/gecko-dev/dom/bindings/BindingUtils.cpp:2426 (0xb4b97b60 libxul.so+0xaf0b60) js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) /home/khuey/dev/gecko-dev/js/src/jscntxtinlines.h:231 (0xb5a6e79c libxul.so+0x19c779c) js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) /home/khuey/dev/gecko-dev/js/src/vm/Interpreter.cpp:461 (0xb5a99c6c libxul.so+0x19f2c6c) Interpret /home/khuey/dev/gecko-dev/js/src/vm/Interpreter.cpp:2558 (0xb5a964d4 libxul.so+0x19ef4d4) RunScript /home/khuey/dev/gecko-dev/js/src/vm/Interpreter.cpp:408 (0xb5a98874 libxul.so+0x19f1874) js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) /home/khuey/dev/gecko-dev/js/src/vm/Interpreter.cpp:480 (0xb5a99c2e libxul.so+0x19f2c2e) js::Invoke(JSContext*, JS::Value const&, JS::Value const&, unsigned int, JS::Value const*, JS::MutableHandle<JS::Value>) /home/khuey/dev/gecko-dev/js/src/vm/Interpreter.cpp:517 (0xb5a9a4d8 libxul.so+0x19f34d8) My understanding of this, is that we create a GLContext for each of the canvas created in the app. There is normally an optimization in the platform at http://mxr.mozilla.org/mozilla-central/source/content/canvas/src/CanvasRenderingContext2D.cpp#841 in order to not create a GLContext if the canvas size is small. I feel like we are not entering the code path that moves us out of SkiaGL here.

Seems like the canvas.width and canvas height are equal to 182 at https://github.com/mozilla-b2g/gaia/blob/master/shared/elements/gaia_grid/js/grid_icon_renderer.js#L43 Which results into multiple SkiaGL context beeing created here. Kyle, this code path is normally triggered only during homescreen initialization the first time it is runned. After that the icons should be cached, and this code path should not be entered (or at least that what happens on my build). Did you take a DMD report after the first run ever of the homescreen ? If so, can you also take one on a second run since different code paths are triggered ?

For test purpose I used a homescreen with the engineering apps + 100 template apps in order to make some icons issues more obvious. I played with column layouts with such a configuration and found (after the first initialization): images/content/raster/used 3 columns layout: 21.71 mo 4 columns layout: 5.77 mo (with a hack to force gridMaxIconSize to returns '84' at https://github.com/mozilla-b2g/gaia/blob/master/shared/elements/gaia_grid/js/grid_layout.js#L104 instead of '189' as it does currently. Which is a diff of 15mo in such a configuration. Probably a lot less in the default configuration (I have not measured yet), but I start to think that we should ship with 4 columns by default and fix gridMaxIconSize to not return crazy things.

I also note that once the app starts, the icons blobs does not all have uncompressed data (which is great), but as soon as the user has started to pan the list, they all have uncompressed data (which is great too). When the app goes into background it seems like the uncompressed data is not released too - which would be great. When the app starts it consumes 3.33mo, and then 5.77mo after scrolling, and it never recovers the startup level.

(In reply to Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please) from comment #44) > 4 columns layout: 5.77 mo (with a hack to force gridMaxIconSize to returns > '84' at > https://github.com/mozilla-b2g/gaia/blob/master/shared/elements/gaia_grid/js/ > grid_layout.js#L104 instead of '189' as it does currently. It seems like 84 is not the proper value to take advantage of devicePixelRatio leaving us with pixelated icons? It does look like our calculations take the adjustment for actively dragged icons. We could probably remove this: https://github.com/mozilla-b2g/gaia/blob/master/shared/elements/gaia_grid/js/grid_layout.js#L106 (the dragdrop.maxActiveScale part)

(In reply to Kevin Grandon :kgrandon from comment #46) > (In reply to Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, > needinfo? please) from comment #44) > > 4 columns layout: 5.77 mo (with a hack to force gridMaxIconSize to returns > > '84' at > > https://github.com/mozilla-b2g/gaia/blob/master/shared/elements/gaia_grid/js/ > > grid_layout.js#L104 instead of '189' as it does currently. > > It seems like 84 is not the proper value to take advantage of > devicePixelRatio leaving us with pixelated icons? It does look like our > calculations take the adjustment for actively dragged icons. We could > probably remove this: > https://github.com/mozilla-b2g/gaia/blob/master/shared/elements/gaia_grid/js/ > grid_layout.js#L106 (the dragdrop.maxActiveScale part) We could even minimize the amount of memory used by creating smaller icons for the default set of apps I believe. It seems like the real display size of the icons will be something like 89 * 0.75 at the end. This is already smaller than 84px (~67). So can we optimize for 4 columns layout by default, and does not scale the icons ? That would let us create slightly smaller icons, and so save some memory bits (probably ~1mo in my test case if we go with 67px icons). It will also avoid a permanent transform on each of the icons.

This may pixelate the actively dragged icon, but it should be fine as it's already got an opacity.

(In reply to Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please) from comment #47) > We could even minimize the amount of memory used by creating smaller icons > for the default set of apps I believe. It seems like the real display size > of the icons will be something like 89 * 0.75 at the end. This is already > smaller than 84px (~67). We already generate icons at the proper size, so we may shave some off of the initial load time. > So can we optimize for 4 columns layout by default, and does not scale the > icons ? That would let us create slightly smaller icons, and so save some > memory bits (probably ~1mo in my test case if we go with 67px icons). > It will also avoid a permanent transform on each of the icons. UX will probably need to sign off on the default setting. We require the following sizes or icons are pixelated (based on a flame): 3 columns require 126px icons and 4 columns require 96px sized icons.

(In reply to Kevin Grandon :kgrandon from comment #49) > (In reply to Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, > needinfo? please) from comment #47) > > We could even minimize the amount of memory used by creating smaller icons > > for the default set of apps I believe. It seems like the real display size > > of the icons will be something like 89 * 0.75 at the end. This is already > > smaller than 84px (~67). > > We already generate icons at the proper size, so we may shave some off of > the initial load time. > As discussed on IRC and as far as I understand, this assumption is not true in a world where we optimize first for 4 columns layout and invalidate the cache on layout changes. > > > So can we optimize for 4 columns layout by default, and does not scale the > > icons ? That would let us create slightly smaller icons, and so save some > > memory bits (probably ~1mo in my test case if we go with 67px icons). > > It will also avoid a permanent transform on each of the icons. > > UX will probably need to sign off on the default setting. We require the > following sizes or icons are pixelated (based on a flame): 3 columns require > 126px icons and 4 columns require 96px sized icons. As a result of the previous statement we can probably go with smaller icons for 4 columns layout, even smaller than 84px which are the smallest size we have for now.

Comment on attachment 8456895 [details] Part 1 - Pull request, do not include dragdrop scale adjustment in icon size Removing reviews as I don't think this patch is enough. Even on a 4 columns display it still consume 12.22mo of memory, more or less twice as more as optimizing for smaller icons. So we need a discussion with UX around the default layout, and if we end up shipping 4 by defaults, we also need to optimize the homescreen icons. We used to ship much smaller icons in previous versions, we can't kill all the memory for some pixels story - which has not been opened in any bugs since v1.1 AFAICT.

Kyle, Tapas, can you measure again with this ? This is a bit of a hack in the current shape, but it should prevent us to enter the SkiaGL codepath for the first run, and should divide the memory use by icons by a few times. It if helps we can explore more changes to optimize for this default config.

Comment on attachment 8456895 [details] Part 1 - Pull request, do not include dragdrop scale adjustment in icon size This is still a patch we need, even if we go to 4 columns. I would like to land this and am opening up an email about moving to 4 columns by default. I will start working on a patch for that as well. For now, let's land this incremental improvement even if we need to do more.

Just wanted to point out that we should avoid skiagl if the canvas is below 128x128 in either dimension. http://dxr.mozilla.org/mozilla-central/source/content/canvas/src/CanvasRenderingContext2D.cpp#844 I'd like to explore having a differ trigger instead. Something where we switch a context from software to skiagl when it's being refresh of rAF for several frames in a row (meaning it looks like a high performance canvas app). But this is for another bug. We should allocate only one GLContext per app for skiagl. So if we get two or more skiagl context in the app they will share some resources.

Comment on attachment 8456895 [details] Part 1 - Pull request, do not include dragdrop scale adjustment in icon size Working on the *proper* patch now.

Hate doing this, but I suppose we don't have many options for 2.0. This patch uses navigator.getFeature() to reduce the visual quality of the homescreen when below 512mb of memory.

Also meant to say that I'm stealing this back from Kyle, unless you have something specific you want to do here Kyle.

The product and UX teams discussed the findings in this bug. We are comfortable with going with a 4 column layout by default to unblock us in 2.0. We'll have to also remove the Settings option to change back to 3. For consistency, can we go to 4 columns for all devices, not just less than 512mb? Once we get everything working with 4 columns, we can explore how to get 3 columns working. If that's into 2.1, so be it.

(In reply to Peter Dolanjski [:pdol] from comment #58) > The product and UX teams discussed the findings in this bug. We are > comfortable with going with a 4 column layout by default to unblock us in > 2.0. We'll have to also remove the Settings option to change back to 3. > > For consistency, can we go to 4 columns for all devices, not just less than > 512mb? > Once we get everything working with 4 columns, we can explore how to get 3 > columns working. If that's into 2.1, so be it. It should be possible to keep the three column layout no problem, this patch will actually make three column layout use less memory due to smaller image sizes, so there should be no issue there. Defaulting all device types to four columns is easy. It's a 1-character change and we already support carrier customization of it.

(In reply to Kevin Grandon :kgrandon from comment #59) > It should be possible to keep the three column layout no problem, this patch > will actually make three column layout use less memory due to smaller image > sizes, so there should be no issue there. Defaulting all device types to > four columns is easy. It's a 1-character change and we already support > carrier customization of it. Whatever combination gets us inline with our 1.4 numbers. For the image sizes, does it affect the actual display size? If so, can you run it by Peter La?

Comment on attachment 8457425 [details] Part 1 - Smaller icon sizes for devices < 256mb memory LGTM, I wrote some comments on github. Thanks

(In reply to Peter Dolanjski [:pdol] from comment #60) > (In reply to Kevin Grandon :kgrandon from comment #59) > > It should be possible to keep the three column layout no problem, this patch > > will actually make three column layout use less memory due to smaller image > > sizes, so there should be no issue there. Defaulting all device types to > > four columns is easy. It's a 1-character change and we already support > > carrier customization of it. > > Whatever combination gets us inline with our 1.4 numbers. For the image > sizes, does it affect the actual display size? If so, can you run it by > Peter La? The actual display size will not be impacted, besides going to 4-columns by default. For low memory devices, we will also not consider display density. This should not matter much as most low-capability devices will not use more than 1x (we will only see this on memory-constrained flames for example). I will send out an update to UX to clarify.

Comment on attachment 8457425 [details] Part 1 - Smaller icon sizes for devices < 256mb memory Can we land that into a specific bug that blocks this one ? I don't think this single patch will solve all the issues. Also we may want to be a bit more aggressive and optimize for 4 columns layout by default in order to remove the scale and use even smaller icons in the app (like ~67px square) - which can be file as a separate bug that also blocks this one and will likely result into more than 1mb of saved memory in my test homescreen with a lot of applications.

Needinfo'ing Kyle again as I would like to see DMD reports for the first run of the homescreen and also subsequent runs. I would also like to understand if Tapas's measurement are for the first run or for subsequent runs as it makes a big differences. In my test homescreen with many apps I see a spike of memory for the first run of the homescreen (~50mb) before it decreases, on subsequent runs the spike only goes to 20mb of memory. Which makes a big difference.

(In reply to Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please) from comment #63) > Comment on attachment 8457425 [details] > Github pull request > > Can we land that into a specific bug that blocks this one ? I don't think > this single patch will solve all the issues. > > Also we may want to be a bit more aggressive and optimize for 4 columns > layout by default in order to remove the scale and use even smaller icons in > the app (like ~67px square) - which can be file as a separate bug that also > blocks this one and will likely result into more than 1mb of saved memory in > my test homescreen with a lot of applications. Also I would like to not wait on the grid.cols setting to start rendering the homescreen if we go 4 columns by default in 2.0. That should make the homescreen loads faster (pure theory) as I feel like it is waiting for some time until this setting is retrieved asynchronously.

(In reply to Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please) from comment #63) > Comment on attachment 8457425 [details] > Github pull request > > Can we land that into a specific bug that blocks this one ? I don't think > this single patch will solve all the issues. Hmm, I feel that this single patch would fix a lot of issues, and that we will get more value from opening individual actionable bugs and blocking them for 2.0+. Also since the implementation has changed dramatically, many of the original numbers are not correct. If startup memory usage is a problem, let's open a 2.0+ blocking bug and track that there? I suppose I just don't really see the value in keeping this metabug around for much longer.

Comment on attachment 8457425 [details] Part 1 - Smaller icon sizes for devices < 256mb memory I am going to call this "part 1" while we measure it, and get some numbers. There may be some improvements we can still make to startup time, but those should be tracked in another bug. It's not as easy as not checking the settings as we still have the option in the settings app, but I will check with UX on that and have an update soon.

(In reply to Kevin Grandon :kgrandon from comment #66) > (In reply to Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, > needinfo? please) from comment #63) > > Comment on attachment 8457425 [details] > > Github pull request > > > > Can we land that into a specific bug that blocks this one ? I don't think > > this single patch will solve all the issues. > > Hmm, I feel that this single patch would fix a lot of issues, and that we > will get more value from opening individual actionable bugs and blocking > them for 2.0+. > Until we have memory reports its hard to said if it will or not. > Also since the implementation has changed dramatically, many of the original > numbers are not correct. If startup memory usage is a problem, let's open a > 2.0+ blocking bug and track that there? I suppose I just don't really see > the value in keeping this metabug around for much longer. The metabug is useful as there may be multiple causes of memory usage regressions. Meta bug are not intended to be solved by one patch :)

Tell me what exactly you want me to measure and I will do it today.

Ok, marking this a meta bug and spinning off a bug for the icon sizing. Also not blocking as it's officially a meta bug. Let's continue to investigate and block on individual bugs that pop up.

(In reply to Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please) from comment #52) > Created attachment 8456934 [details] [diff] [review] > hack.homescreen.layout.patch > > Kyle, Tapas, > > can you measure again with this ? > > This is a bit of a hack in the current shape, but it should prevent us to > enter the SkiaGL codepath for the first run, and should divide the memory Sorry for late reply. It seems like we have multiple patches for this bug. I will try these today and see how much gain we have with these patches. > use by icons by a few times. > It if helps we can explore more changes to optimize for this default config.

(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #69) > Tell me what exactly you want me to measure and I will do it today. On your side I'm curious about DMD reports for first run, and a second run. After GC would be nice.

With which patches?

(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #73) > With which patches? Kevin's patch or my patch does basically the same thing. Mine force a return value but Kevin's patch, does it properly. So the one you want. The big differences between the 2 patches is that if you use Kevin's patch and runs it on top of a device that has more than 512mb of memory, you may not see all the benefits. In order to invalidate the cache, you also need to flash the device with flash.sh, so it will delete the content of /data/local/storage/persistent for the homescreen. I'm interested in the DMD report after this first run. Then if you restart the device again and take a new reports, we will hit the icons cache of the homescreen, and the memory consumption should be lower than the first run. I'm interested in the DMD report after this run as well. Thanks!

Vivien's patch looks good. Measurements in a few.

(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #75) > Vivien's patch looks good. Measurements in a few. Awesome. If you do get a chance, please also check mine. The image size should actually be slightly smaller than Vivien's, 64px instead of 84px on a HVGA screen.

(Or you could hardcode Vivien's to return 64)

http://people.mozilla.org/~khuey/1029902-measurements.tgz The highlights: The difference on 1st run with Vivien's patch is -5.71 MB (100.0%) -- explicit ├──-4.63 MB (81.02%) ── heap-unclassified ├──-0.79 MB (13.85%) -- images/content/raster/used ├──-0.79 MB (13.78%) -- dom/memory-file-data └───0.49 MB (-8.66%) ++ (5 tiny) The difference on second run with Vivien's patch is: -5.69 MB (100.0%) -- explicit ├──-4.59 MB (80.74%) ── heap-unclassified ├──-0.79 MB (13.91%) -- images/content/raster/used ├──-0.35 MB (06.12%) -- heap-overhead └───0.04 MB (-0.77%) ++ (5 tiny) I didn't take DMD logs. I can get those if you want them, it's just more annoying because I have to rebuild Gecko from scratch (my DMD build is for master). These were taken on top of Gecko: 9d7c9c2c29bc4021045ecb1ba2a1eaeb53ce26d6 Gaia: 9977a02ea62ba96425a1cc4d1bfb54a909f68905

(In reply to Kevin Grandon :kgrandon from comment #77) > (Or you could hardcode Vivien's to return 64) Sure, I'll do that.

At 64 px. -6.20 MB (100.0%) -- explicit ├──-4.61 MB (74.29%) ── heap-unclassified ├──-0.90 MB (14.59%) ++ images/content/raster/used ├──-0.90 MB (14.52%) ++ dom/memory-file-data └───0.21 MB (-3.40%) ++ (5 tiny) Let me know if you want the full logs, I can get those.

Kyle - thanks for grabbing those reports. It seems this bug was opened with 7.4, and once we land that patch we will now be at ~6.20. I don't think there's very much more we can from inside the homescreen app itself. Do you think more action is needed here? If so, what's next? Would getting a DMD of the 4.61mb of heap-unclassified help us?

Kevin, those are diffs, not absolute values.

That makes sense. So is the original number an absolute value? I'll try to measure this on my own as well, just want to make sure I follow the same methodologies so we can have comparable numbers.

No, the original was a diff too.

We should benchmark it against 1.4 again but I'm pretty confident that these patches get us into good shape.

(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #78) > http://people.mozilla.org/~khuey/1029902-measurements.tgz > > The highlights: > > The difference on 1st run with Vivien's patch is > -5.71 MB (100.0%) -- explicit > ├──-4.63 MB (81.02%) ── heap-unclassified > ├──-0.79 MB (13.85%) -- images/content/raster/used > ├──-0.79 MB (13.78%) -- dom/memory-file-data > └───0.49 MB (-8.66%) ++ (5 tiny) > > The difference on second run with Vivien's patch is: > -5.69 MB (100.0%) -- explicit > ├──-4.59 MB (80.74%) ── heap-unclassified > ├──-0.79 MB (13.91%) -- images/content/raster/used > ├──-0.35 MB (06.12%) -- heap-overhead > └───0.04 MB (-0.77%) ++ (5 tiny) > I am surprised that there is such a diff for heap-unclassified and such a small one for images/content/raster/used. Is it possible that on 2.0 the image code is not correctly instrumented and so the heap-unclassified is some uncompressed-heap for blobs ? > I didn't take DMD logs. I can get those if you want them, it's just more > annoying because I have to rebuild Gecko from scratch (my DMD build is for > master). > The DMD build on master would be enough. There is no real differences for the homescreen between master and 2.0 - most of the patches has been uplifted. The delta between those 2 versions will grow in the coming weeks though, so now is a good time to look at master to see things that happens in 2.0.

(In reply to Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please) from comment #86) > I am surprised that there is such a diff for heap-unclassified and such a > small one for images/content/raster/used. > Is it possible that on 2.0 the image code is not correctly instrumented and > so the heap-unclassified is some uncompressed-heap for blobs ? I haven't been following this bug closely but drive-by comment: Timothy recently found that in some cases images don't get tracked properly and end up in heap-unclassified. See https://bugzilla.mozilla.org/show_bug.cgi?id=1033679#c19. That might be what's happening here.

Measurements with DMD on 2.0, same revs as before: http://people.mozilla.org/~khuey/1029902-measurements-with-dmd.tgz You can see that basically all of the GL related entries are gone in the 'after' log.

(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #88) > Measurements with DMD on 2.0, same revs as before: > http://people.mozilla.org/~khuey/1029902-measurements-with-dmd.tgz > > You can see that basically all of the GL related entries are gone in the > 'after' log. Thanks for the reports. I don't see anything else obvious as memory issue in the 'after' log. Not sure what else can be done in this specific bug?

I think after the modifications in this bug we are good.

Why does a small change in icon size cause such a huge memory overhead that we can't ship with 3 column icons?

AIUI, the size of the canvas used to produce/draw them triggers a different graphics subsystem that uses a bunch more memory.

(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #90) > I think after the modifications in this bug we are good. I think we're looking much better here as well. We're tracking separate issues here, is it worthwhile to keep this bug open? (In reply to Andreas Gal :gal from comment #91) > Why does a small change in icon size cause such a huge memory overhead that > we can't ship with 3 column icons? I've filed bug 1041112 so we can do some investigation.

(In reply to Andreas Gal :gal from comment #91) > Why does a small change in icon size cause such a huge memory overhead that > we can't ship with 3 column icons? That's because it is not a small change in the size of the icons. We have been from 182x182px to 64x64px. At first run the 182x182px icons are cached, and a canvas is used to alter them a little bit. The canvas backend check for the size of the canvas, and if this one is bigger than 128px in both dimension, it uses skiaGL as the backend. But that said, this stuff is supposed to be true only for the first run of the homescreen, when the cache is generated. After that we are not supposed to enter this codepath on subsequent runs. But with this patch the size of icons is permanently reduced by 118*118*number_of_informations_by_pixels. Then you multiply this by the number of icons on the homescreen (once you have panned otherwise not all the icons are decoded yet), and you end up with megabytes.

(In reply to Kevin Grandon :kgrandon from comment #93) > (In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #90) > > I think after the modifications in this bug we are good. > > I think we're looking much better here as well. We're tracking separate > issues here, is it worthwhile to keep this bug open? > I'm starting to think that this bug does not need to be opened anymore. I first looked at it as a meta bug but looking at #c0 it seems like it was related to head-unclassified that we did optimize here. Let's close it.

Closing this bug based on comment 90 and comment 95.

clearing NI and waiting to see fix from all 5 dependent bugs.