Camera app shows 7244 KB USS and 8920 KB PSS in FFOS 1.3 But it shows following 13068 KB USS and 14829 KB PSS in FFOS 2.0 This data is collected by running |b2g-procrank| command on msm8610 device. This can become a bottleneck for 256MB FFOS 2.0 device.

Justin, Can you help investigate this on flame please? Thanks Hema

(In reply to Tapas Kumar Kundu from comment #0) > Camera app shows 7244 KB USS and 8920 KB PSS in FFOS 1.3 > > But it shows following 13068 KB USS and 14829 KB PSS in FFOS 2.0 > > This data is collected by running |b2g-procrank| command on msm8610 device. > > This can become a bottleneck for 256MB FFOS 2.0 device. Do you happen to have mem usage numbers for FFOS 1.4 ? I'm going to start looking into this today to see if I can find any contributing factors that may have changed in 2.0, but I would almost think that a memory increase would be expected when comparing v1.3 and v1.4/v2.0. The Camera app starting in v1.4 is almost a completely new app due to refactoring and a ton of new features. Whereas the Camera in v1.3 is extremely basic (no zoom, no tap-to-focus, no face detection, no settings menu, etc.).

I don't believe this is a v2.0 regression. If anything, it is a v1.4 regression. However, the Camera (starting with v1.4) is an entirely different app than the one that shipped in v1.3. Also, in testing using `b2g-info` (which replaced b2g-procrank), my PSS numbers in v2.0 are actually slightly *lower* than the ones in v1.4. Here's the results I got (all numbers reported are on the Flame device): v1.3: 11.1 MB v1.4: 17.9 MB v2.0: 17.1 MB v2.1: 17.5 MB

I'll get some memory reports for this.

Here's the PSS numbers I got for Hamachi (averaging 5 runs by running `b2g-info`): v1.3: 10.5 MB v1.4: 16.8 MB v2.0: 16.9 MB v2.1: 16.8 MB It seems like the app's memory usage is pretty consistent between v1.4 and v2.1, but with a fairly consistent ~6 MB increase between v1.3 and v1.4. This is likely due to the increase in size of the new Camera application in v1.4.

Mike: See the above Comment 3 and Comment 5 -- Is this increase between v1.3 and v1.4 acceptable? The codebase for the Camera has probably increased 4x in v1.4 to accommodate all of the new features.

Just added a See Also for Bug 1029856 -- Appears to be a memory leak related to HWC when running the Camera app. This may account for some of the increase in v1.4

A ~50% increase in the PSS does seem like something we should look at.

oops

(In reply to Justin D'Arcangelo [:justindarc] from comment #2) > (In reply to Tapas Kumar Kundu from comment #0) > > Camera app shows 7244 KB USS and 8920 KB PSS in FFOS 1.3 > > > > But it shows following 13068 KB USS and 14829 KB PSS in FFOS 2.0 > > > > This data is collected by running |b2g-procrank| command on msm8610 device. > > > > This can become a bottleneck for 256MB FFOS 2.0 device. > > Do you happen to have mem usage numbers for FFOS 1.4 ? > Numbers of FFOS 1.4 is matching with numbers from FFOS 2.0. Both are very close. #comment 8 seems to me interesting. We may gain something by digging in that direction.

(In reply to Tapas Kumar Kundu from comment #10) > Numbers of FFOS 1.4 is matching with numbers from FFOS 2.0. Both are very > close. Shouldn't this bug then be re-classified as a 1.4+ blocker instead of 2.0+?

(In reply to Justin D'Arcangelo [:justindarc] from comment #11) > > Shouldn't this bug then be re-classified as a 1.4+ blocker instead of 2.0+? The fix is not required on v1.4 from our POV

(In reply to Justin D'Arcangelo [:justindarc] from comment #11) > (In reply to Tapas Kumar Kundu from comment #10) > > Numbers of FFOS 1.4 is matching with numbers from FFOS 2.0. Both are very > > close. > > Shouldn't this bug then be re-classified as a 1.4+ blocker instead of 2.0+? Let me ask Wayne to find out if our partner wants this or not.

Let's keep this on 2.0+ for now given we have 1.4+ on bug 1029856, where an actual problem is seen.

(In reply to Wayne Chang [:wchang] from comment #14) > Let's keep this on 2.0+ for now given we have 1.4+ on bug 1029856, where an > actual problem is seen. Looks like Bug 1029856 is resolved -- Going to re-check the numbers today to see if we still have a regression in v1.4+

Bug 1029856 has landed, but it doesn't seem to have affected Camera's memory utilization. I'll dig around a little more on the JS side of things to see if there's anything that can be done to reduce memory consumption.

(In reply to Eric Rahm [:erahm] from comment #4) > I'll get some memory reports for this. Eric: Have you made any progress on this?

Justin: does our preview stream match the screen size or are we taking a large one and scaling it down to match? You might try forcing the code to use an unrealistically small preview stream to see if that has any impact on memory use. (Or check memory usage when viewing a photo without the preview stream running.) There have been substantial gecko changes since 1.3 also. Mike: can you think of any gecko changes that would cause substantially increased memory usage by the camera? It might also be interesting to know what the % increase in JS source code is between 1.3 and 2.0. Really, though, I think we shouldn't be speculating based on b2g-ps and procrank output. We need about:memory reports for 1.3 vs 2.0 here.

(In reply to David Flanagan[OOO until July 14] [:djf] from comment #18) > Justin: does our preview stream match the screen size or are we taking a > large one and scaling it down to match? You might try forcing the code to > use an unrealistically small preview stream to see if that has any impact on > memory use. (Or check memory usage when viewing a photo without the preview > stream running.) > > There have been substantial gecko changes since 1.3 also. Mike: can you > think of any gecko changes that would cause substantially increased memory > usage by the camera? > > It might also be interesting to know what the % increase in JS source code > is between 1.3 and 2.0. > > Really, though, I think we shouldn't be speculating based on b2g-ps and > procrank output. We need about:memory reports for 1.3 vs 2.0 here. I already tried forcing the smallest-possible preview stream on my Flame yesterday and there was very little change in the procrank reports. I'll do a JS source code size analysis between 1.3 and 1.4 later today. The size of the codebase has at least doubled (if not more) to accommodate all of the new features.

(In reply to David Flanagan[OOO until July 14] [:djf] from comment #18) > Really, though, I think we shouldn't be speculating based on b2g-ps and > procrank output. We need about:memory reports for 1.3 vs 2.0 here. Also, note that this bug is filed specifically to address the increase in the procrank data. I agree that we really do need to see the memory reports to determine if there is an unusually-large increase in utilization. I also think that it would be unreasonable to assume that we would have zero increase between 1.3 and 1.4 given that they are not even close to the same app.

(In reply to David Flanagan[OOO until July 14] [:djf] from comment #18) > > There have been substantial gecko changes since 1.3 also. Mike: can you > think of any gecko changes that would cause substantially increased memory > usage by the camera? The changes and new features in Gecko since 1.3 would have an effect measured in tens-of-kilobytes, maybe 100KiB at the -very- outside. I can try to code up a minimal Camera app (viewfinder-only) that works on all of our trees to get a more solid idea.

Here's a(n untested) mini-Camera app. All it does is open the viewfinder, and it should work on all versions of the CameraControl API. Need to figure out how to push it onto a real device.

Update: - create a manifest, move script to separate file, add icon - fix getCamera() signature checking This does nothing but open the camera and throw up the viewfinder.

Gaia d7a517f0bde32072f1799e4a47ea34c6d786c287 Gecko https://hg.mozilla.org/mozilla-central/rev/b7b20af4a4fb BuildID 20140703040209 Version 33.0a1 ro.build.version.incremental=108 ro.build.date=Tue Jun 10 19:40:40 CST 201 14:16:03 ➜ ~ adb shell b2g-procrank APPLICATION PID Vss Rss Pss Uss cmdline b2g 289 118468K 92508K 75201K 66968K /system/b2g/b2g Homescreen 1131 50604K 43628K 25879K 18812K /system/b2g/plugin-container Minimal Camera 1438 44796K 31224K 17441K 14404K /system/b2g/plugin-container Smart Collectio 1149 22792K 22780K 9999K 7416K /system/b2g/plugin-container Built-in Keyboa 988 22176K 22164K 9549K 7048K /system/b2g/plugin-container (Preallocated a 1678 19020K 19008K 7902K 5840K /system/b2g/plugin-container (Nuwa) 917 20076K 20064K 6665K 2364K /system/b2g/plugin-container ------ ------ ------ 183529K 148224K TOTAL ----- Gaia 7ad00b0bd84d5d97e0801e3b3ceaac33fcd90e05 Gecko https://hg.mozilla.org/releases/mozilla-aurora/rev/e87f7b398fce BuildID 20140703000248 Version 32.0a2 ro.build.version.incremental=108 ro.build.date=Tue Jun 10 19:40:40 CST 2014 14:22:34 ➜ ~ adb shell b2g-procrank APPLICATION PID Vss Rss Pss Uss cmdline b2g 304 131492K 91224K 73359K 65200K /system/b2g/b2g Homescreen 1094 51040K 44004K 25790K 19280K /system/b2g/plugin-container Minimal Camera 1305 43540K 29968K 15941K 13248K /system/b2g/plugin-container Settings 1128 34556K 29584K 15281K 12520K /system/b2g/plugin-container Smart Collectio 1107 22740K 22728K 9736K 7572K /system/b2g/plugin-container Built-in Keyboa 980 21856K 21844K 9048K 6904K /system/b2g/plugin-container (Preallocated a 1466 18888K 18876K 7386K 5572K /system/b2g/plugin-container (Nuwa) 917 20484K 20472K 6551K 2212K /system/b2g/plugin-container ------ ------ ------ 199039K 163000K TOTAL ----- Gaia f250750c9b8381560fa33b2383737736e4dbdff5 Gecko https://hg.mozilla.org/releases/mozilla-b2g30_v1_4/rev/4c05c3fb440d BuildID 20140703000230 Version 30.0 ro.build.version.incremental=108 ro.build.date=Tue Jun 10 19:40:40 CST 2014 ...apparently |navigator.mozCameras| doesn't exist in this build? (Though the camera app works just fine.) Until I get this sorted out, there is definitely an across-the-board numbers increase in memory usage even for the Minimal Camera app between v2.1 (m-c) and v2.0 (aurora) -- though there are no significant changes in the Gecko camera code between these two versions.

I'm unable to get a report for 1.3 currently (there doesn't appear to be flame support), I'm attaching a memory report for 1.4. There's nothing really outstanding to me in it, it would probably be better with a diff. If someone has has access to a 1.3 device you can get a memory report with the following command: MOZ_IGNORE_NUWA_PROCESS=1 tools/get_about_memory.py --minimize And if it's not a flame we'll probably want a 1.4 report from that device as well.

Mike: I've wrapped your mini camera app up into an app template generated by the shiny new WebIDE :-D You can add this and deploy it to the device using WebIDE in Nightly. Also, there was an issue with the API version detection in your code that is now fixed.

Here are the procrank (b2g-info) numbers for the Mike's mini camera app (same app, only opens viewfinder on v1.3 through v2.1): NAME PID PPID CPU(s) NICE USS PSS RSS VSIZE OOM_ADJ USER 1.3 Camera Test 2197 1482 0.8 1 5.5 8.9 20.0 75.0 2 u0_a2197 1.4 Camera Test 1510 897 0.9 1 6.1 10.2 20.2 76.0 2 u0_a1510 2.0 Camera Test 1922 908 1.0 1 6.1 8.4 21.6 78.4 2 u0_a1922 2.1 Camera Test 2169 1691 1.1 1 6.4 9.0 22.1 80.7 2 u0_a2169 As you can see, not much of a change between v1.3 and v1.4 (slight increase). I still need to try and get a v1.3 memory report from either my Flame or perhaps I'll just try comparing v1.3 and v1.4 on a Hamachi. Once I can compare the memory reports with v1.3, we might be able to get to the bottom of the cause of the increase. Because there isn't much of a difference in terms of memory usage in the mini camera app (viewfinder only), I'm going to assume that our increase can be attributed to the UI refresh (introduced in v1.4) as well as the entire-app overhaul (all the new features were introduced in v1.4 as well which also increased the size of the codebase significantly).

I generated this memory report using the v1.3 build that is included with the v122 base build.

This diff show's a lot of small increases. Of note (to me at least): │ ├──1.51 MB (36.48%) -- active/window(app://camera.gaiamobile.org/index.html) │ │ ├──1.25 MB (30.18%) -- js-compartment(app://camera.gaiamobile.org/index.html) ├──0.82 MB (19.86%) -- js-non-window │ │ ├──0.18 MB (04.34%) -- strings │ ├──0.50 MB (11.99%) -- runtime │ │ ├──0.23 MB (05.60%) ── script-data │ │ ├──0.11 MB (02.59%) -- script-sources ├──0.46 MB (11.15%) -- media │ ├──0.25 MB (06.09%) ── libogg [+] │ ├──0.11 MB (02.69%) ── decoded/audio │ └──0.10 MB (02.37%) ── resources [+]

Here is a diff of the about:memory reports for the Camera app between v1.3 and v1.4

(In reply to Eric Rahm [:erahm] from comment #29) > Created attachment 8453228 [details] > Camera 1.3 -> 1.4 memory report diff > > This diff show's a lot of small increases. Of note (to me at least): > > │ ├──1.51 MB (36.48%) -- > active/window(app://camera.gaiamobile.org/index.html) > │ │ ├──1.25 MB (30.18%) -- > js-compartment(app://camera.gaiamobile.org/index.html) > > ├──0.82 MB (19.86%) -- js-non-window > │ │ ├──0.18 MB (04.34%) -- strings > │ ├──0.50 MB (11.99%) -- runtime > │ │ ├──0.23 MB (05.60%) ── script-data > │ │ ├──0.11 MB (02.59%) -- script-sources > > ├──0.46 MB (11.15%) -- media > │ ├──0.25 MB (06.09%) ── libogg [+] > │ ├──0.11 MB (02.69%) ── decoded/audio > │ └──0.10 MB (02.37%) ── resources [+] Eric: Is it safe to assume that this is due to the large number of new features and UI refresh? (the v1.3 and v1.4 Camera apps are not even close to the same thing).

(In reply to Justin D'Arcangelo [:justindarc] from comment #31) > > Eric: Is it safe to assume that this is due to the large number of new > features and UI refresh? (the v1.3 and v1.4 Camera apps are not even close > to the same thing). I think overall that makes sense. The main increases we're seeing are all JS related. It's possible we can reduce the overall script size which could be a win, and it's possible we're keeping JS-related things around longer than we need to. We can probably do some things to cleanup the media usage (we might have some media elements hanging around that we don't need). Someone from the camera team should probably chime in on this.

(In reply to Eric Rahm [:erahm] from comment #32) > (In reply to Justin D'Arcangelo [:justindarc] from comment #31) > > > > Eric: Is it safe to assume that this is due to the large number of new > > features and UI refresh? (the v1.3 and v1.4 Camera apps are not even close > > to the same thing). > > I think overall that makes sense. The main increases we're seeing are all JS > related. It's possible we can reduce the overall script size which could be > a win, and it's possible we're keeping JS-related things around longer than > we need to. We can probably do some things to cleanup the media usage (we > might have some media elements hanging around that we don't need). > > Someone from the camera team should probably chime in on this. Well, that would be myself, Diego or Wilson. We are usually discarding things (media elements, etc.) that aren't in use.. but I can try a couple things out to see if it helps any. For the most part though, we throw away views when they're done being used and we lazy-load as much as possible. I'm not sure a whole lot can be done to reduce the increase.

(In reply to Eric Rahm [:erahm] from comment #29) > Created attachment 8453228 [details] > Camera 1.3 -> 1.4 memory report diff > > This diff show's a lot of small increases. Of note (to me at least): > > │ ├──1.51 MB (36.48%) -- > active/window(app://camera.gaiamobile.org/index.html) > │ │ ├──1.25 MB (30.18%) -- > js-compartment(app://camera.gaiamobile.org/index.html) > > ├──0.82 MB (19.86%) -- js-non-window > │ │ ├──0.18 MB (04.34%) -- strings > │ ├──0.50 MB (11.99%) -- runtime > │ │ ├──0.23 MB (05.60%) ── script-data > │ │ ├──0.11 MB (02.59%) -- script-sources > > ├──0.46 MB (11.15%) -- media > │ ├──0.25 MB (06.09%) ── libogg [+] > │ ├──0.11 MB (02.69%) ── decoded/audio > │ └──0.10 MB (02.37%) ── resources [+] Can we generate a similar diff for 1.3->2.0 or 1.4->2.0 to see if we can analyze this better which was the original idea behind filling this bug from CAF ?

(In reply to bhavana bajaj [:bajaj] [NOT reading Bugmail, needInfo please] from comment #34) > (In reply to Eric Rahm [:erahm] from comment #29) > > Created attachment 8453228 [details] > > Camera 1.3 -> 1.4 memory report diff > > > > This diff show's a lot of small increases. Of note (to me at least): > > > > │ ├──1.51 MB (36.48%) -- > > active/window(app://camera.gaiamobile.org/index.html) > > │ │ ├──1.25 MB (30.18%) -- > > js-compartment(app://camera.gaiamobile.org/index.html) > > > > ├──0.82 MB (19.86%) -- js-non-window > > │ │ ├──0.18 MB (04.34%) -- strings > > │ ├──0.50 MB (11.99%) -- runtime > > │ │ ├──0.23 MB (05.60%) ── script-data > > │ │ ├──0.11 MB (02.59%) -- script-sources > > > > ├──0.46 MB (11.15%) -- media > > │ ├──0.25 MB (06.09%) ── libogg [+] > > │ ├──0.11 MB (02.69%) ── decoded/audio > > │ └──0.10 MB (02.37%) ── resources [+] > > Can we generate a similar diff for 1.3->2.0 or 1.4->2.0 to see if we can > analyze this better which was the original idea behind filling this bug from > CAF ? Comment 0 reports an increase in the b2g-procrank numbers between v1.3 and v2.0. However, the procrank numbers for v1.4, v2.0 and v2.1 are *very* stable. The reported increase occurs between v1.3 and v1.4. I'm not sure how comparing with v2.0 is going to reveal anything different than what the v1.4 about:memory diff is already showing. Especially since v1.4 is when the UI refresh and majority of new features landed, not v2.0.

Increases in "script-data" and "script-sources" are almost certainly due to having more JS code. And it's likely that the "strings" increase has the same cause, though I can't say definitively.

This patch gives me a memory savings of around 0.5 MB in v1.4. It converts all of our sounds to .opus format (which is supposed to be slightly more efficient than .ogg). It also discards any HTML template strings that are no longer used in views after initialization (there was an increase in the amount of memory used for strings in v1.4).

How is script-sources for app://camera.gaiamobile.org/js/main.js 180 KB when http://mxr.mozilla.org/gaia/source/apps/camera/js/main.js is less than 1 KB? The amount of memory we're spending on function objects seems kind of high too. 18% heap-unclassified. Beyond that I don't think there's much actionable.

Maybe the "require" stuff at the top is pulling in more source? I have no idea how that works.

(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #38) > How is script-sources for app://camera.gaiamobile.org/js/main.js 180 KB when > http://mxr.mozilla.org/gaia/source/apps/camera/js/main.js is less than 1 KB? > The Makefile concatenates all critical startup path sources into a single large main.js. Everything else gets lazy-loaded with RequireJS after the critical startup path finishes (once the viewfinder is live and the basic camera controls are accessible). > The amount of memory we're spending on function objects seems kind of high > too. > > 18% heap-unclassified. > Yes, this can definitely be attributed to new features. Everything is broken out into maintainable components; so, yes, there are a lot of functions. > Beyond that I don't think there's much actionable. I agree. There are no obvious, alarming leaks as far as I can tell. We can also be fairly certain that the Gecko-side Camera API changes have not introduced any significant memory regressions from running Mike's mini-camera app (viewfinder only, no controls) across v1.3 - v2.1 (see Comment 27). Not to underplay the importance of the additional memory overhead, but... the original Camera app in v1.3 (and earlier) was *extremely* barebones (only take photo, record video and display thumbnail strip). We now have an all new UI design, new preview gallery, real-time pinch-to-zoom capability, HDR, self-timer, settings menu, and a lot more other small odds and ends. The total increase is around 4 MB. Given that the previous app was only using around 8 MB, it appears like a 50% increase. The "50% increase" sounds alarming, but given that the previous app was extremely limited, it also had a *very* low memory requirement. Because of that, I think that this "regression" seems more drastic than it probably is when you look at it from a percentages point-of-view. The bottom line is: if we absolutely need to ship a "low-memory" version, then we will need to eliminate features from the build. Likely candidates for features to strip out would be: pinch-to-zoom and the settings menu (which includes HDR, self-timer and grid lines toggles). However, if we can live with a ~3.5 MB increase, then we can land my patch that removes some of the strings overhead and optimizes our sound assets.

Comment on attachment 8453974 [details] pull-request (v1.4) Just made a couple of suggestions to move some function calls to a different place. Almost ready.

I think comment 40 addresses the needinfo from comment 38. Thanks, Justin.

Comment on attachment 8453974 [details] pull-request (v1.4) Diego: Please re-review when you get a chance. Addressed your PR comments. Thanks!

There's not a ton of big ticket items in the DMD report. I did notice a fair amount of font-related entries in the top 25 accounting for ~172KB.

(In reply to Eric Rahm [:erahm] from comment #44) > Created attachment 8454586 [details] > DMD, GC, and CC logs > > There's not a ton of big ticket items in the DMD report. I did notice a fair > amount of font-related entries in the top 25 accounting for ~172KB. We do use an icon-font for all the icons in the app. We currently inline the icon font as a base-64 encoded string in the CSS. Are there any known memory-related pros/cons to doing that versus loading via separate request?

Landed on v1.4: https://github.com/mozilla-b2g/gaia/commit/b7d36622c7df92c976c37520ccab25199c7ada91 Eric/Hema: Who makes the ultimate decision as to whether or not to call this a blocking regression? This patch should bring a little bit of memory savings to Camera, but it would be impossible to not have any increase going from v1.3 to v1.4. I will leave this bug open until I hear your input on this.

(In reply to Justin D'Arcangelo [:justindarc] from comment #46) > Landed on v1.4: > > https://github.com/mozilla-b2g/gaia/commit/ > b7d36622c7df92c976c37520ccab25199c7ada91 > > Eric/Hema: Who makes the ultimate decision as to whether or not to call this > a blocking regression? This patch should bring a little bit of memory > savings to Camera, Could you please give us a PR for ffos 2.0 ? > but it would be impossible to not have any increase going > from v1.3 to v1.4. I will leave this bug open until I hear your input on > this. Could you please send us a test patch which will disable certain features in current gaia camera app in FFOS 2.0 to remove this memory regression fully ? Based on your test patch, we can decide whether to keep those features in v2.0 or not. Thanks a lot for your help.

(In reply to Justin D'Arcangelo [:justindarc] from comment #46) > Landed on v1.4: > > https://github.com/mozilla-b2g/gaia/commit/ > b7d36622c7df92c976c37520ccab25199c7ada91 > > Eric/Hema: Who makes the ultimate decision as to whether or not to call this > a blocking regression? This patch should bring a little bit of memory > savings to Camera, but it would be impossible to not have any increase going > from v1.3 to v1.4. I will leave this bug open until I hear your input on > this. Justin, We have a couple of memory regression bugs opened tracking zoom, HDR etc. on flame (configured at 273MB) and QRD (at 256MB). Diego has put in a patch here: https://bugzilla.mozilla.org/show_bug.cgi?id=1024692 to restrict to 3MP images and Mike is working on disabling HDR for devices less than 512MB mem (https://bugzilla.mozilla.org/show_bug.cgi?id=1036637). The new camera app since 1.4 has several new features that attribute to the increase. We will probably need to disable mem intense camera features such as pinch-to-zoom, HDR on low-mem devices. Can you help run the memory report with patches from 1024692 and 1036637 and your patch on 2.0 Flame (273MB)? The results will help decide. Thanks Hema

Updating title/tracking flags to be clearer about which versions are affected and when the memory utilization increased (it didn't start in v2.0). Going to migrate this v1.4 patch to v2.0 and re-run the memory reports incorporating the patches from Bug 1024692 and Bug 1036637 to see where we're at now.

From purely a memory usage (and I guess usability) perspective it seems like it will be worth the effort to add the ability to pref off certain high memory camera features. Perhaps it would make sense to track that in another bug?

(In reply to Justin D'Arcangelo [:justindarc] from comment #45) > (In reply to Eric Rahm [:erahm] from comment #44) > > Created attachment 8454586 [details] > > DMD, GC, and CC logs > > > > There's not a ton of big ticket items in the DMD report. I did notice a fair > > amount of font-related entries in the top 25 accounting for ~172KB. > > We do use an icon-font for all the icons in the app. We currently inline the > icon font as a base-64 encoded string in the CSS. Are there any known > memory-related pros/cons to doing that versus loading via separate request? A bit of a tangent: It looks like bug 1008458 was going to allow us to load the icon font from the system, which seems like it would have been a good thing memory-wise but that got nixed. In bug 1030829 we added the ability to at least look up cached fonts by data URIs, so in theory we're avoiding the loading overhead (as long as the URIs match of course). In general I think that's a loss for us memory-wise as we're now keeping the base64-encoded font in memory (twice I'm guessing, once in the font cache and once in the CSS). Also that just landed in 33 so, we're not going to see the performance win in 1.4/2.0 unless it gets uplifted. On to the original question: embedding as base64 vs dynamically loading. I would guess loading via a separate request is better memory-wise, but worse performance-wise. Without memory reports comparing the two or a more intimate knowledge of font-loading that's bout all I can say. Johnathan seems to know a bit more about fonts (or at least has some background on the bugs I mentioned), perhaps he could weigh in on this.

(In reply to Tapas Kumar Kundu from comment #47) > (In reply to Justin D'Arcangelo [:justindarc] from comment #46) > > Landed on v1.4: > > > > https://github.com/mozilla-b2g/gaia/commit/ > > b7d36622c7df92c976c37520ccab25199c7ada91 > > > > Eric/Hema: Who makes the ultimate decision as to whether or not to call this > > a blocking regression? This patch should bring a little bit of memory > > savings to Camera, > > Could you please give us a PR for ffos 2.0 ? > > > > but it would be impossible to not have any increase going > > from v1.3 to v1.4. I will leave this bug open until I hear your input on > > this. > > Could you please send us a test patch which will disable certain features in > current gaia camera app in FFOS 2.0 to remove this memory regression fully ? > > Based on your test patch, we can decide whether to keep those features in > v2.0 or not. > Thanks a lot for your help. Even if we disable features and don't load the modules required for those features, we still incur some additional memory overhead from the overall increase in size of the codebase as well as parts of the new UI design that landed in v1.4. Simply put, the app is more sophisticated than it was in v1.3, so while we may be able to reduce the memory usage by disabling some features, the amount by which we will save in disabling them will not likely get us all the way back to our v1.3 numbers.

(In reply to Eric Rahm [:erahm] from comment #51) > (In reply to Justin D'Arcangelo [:justindarc] from comment #45) > > (In reply to Eric Rahm [:erahm] from comment #44) > > > Created attachment 8454586 [details] > > > DMD, GC, and CC logs > > > > > > There's not a ton of big ticket items in the DMD report. I did notice a fair > > > amount of font-related entries in the top 25 accounting for ~172KB. > > > > We do use an icon-font for all the icons in the app. We currently inline the > > icon font as a base-64 encoded string in the CSS. Are there any known > > memory-related pros/cons to doing that versus loading via separate request? > > A bit of a tangent: > It looks like bug 1008458 was going to allow us to load the icon font from > the system, which seems like it would have been a good thing memory-wise but > that got nixed. In bug 1030829 we added the ability to at least look up > cached fonts by data URIs, so in theory we're avoiding the loading overhead > (as long as the URIs match of course). In general I think that's a loss for > us memory-wise as we're now keeping the base64-encoded font in memory (twice > I'm guessing, once in the font cache and once in the CSS). Yes, unfortunately; although the preloading of the font into cache happens before the Nuwa fork, so my understanding is that all processes should be sharing the same memory pages for this. AFAICT the camera app isn't currently taking advantage of this, however; it's simply loading its own embedded gaia-icons, not using the preloaded font from the system. > Also that just > landed in 33 so, we're not going to see the performance win in 1.4/2.0 > unless it gets uplifted. > > On to the original question: embedding as base64 vs dynamically loading. I > would guess loading via a separate request is better memory-wise, but worse > performance-wise. That's correct, in general. Linking to a separate file allows FreeType to mmap() the font, whereas using embedded base64 means we have to decode this to an in-memory buffer (in addition to the base64 data hanging around in the resource's URI) that we can then hand to FreeType in its entirety. Linking to a separate file means that the font doesn't get loaded at all until layout encounters a character styled to use that font; then it kicks off an asynchronous resource fetch. Later, once the font is loaded, we reflow again to render using it. So there's some potential lag here. OTOH, a base64 data: URI is loaded synchronously when we need the font, so it blocks that reflow but does not trigger a re-reflow later. And if the data: URI corresponds to a pre-cached font (as per bug 1030829), the load is almost instant.

Patch for v2.0 branch, carrying R+ forward.

Comment on attachment 8456246 [details] pull-request (v2.0) Diego: This patch for v2.0 has changed a little more than the previous one you reviewed for v1.4, so I'm resetting the flag to R? -- I was able to get about ~0.5 MB savings by *NOT* pre-loading our sound assets. The total savings from this patch is now about 0.77 MB: -0.77 MB (100.0%) -- explicit ├──-0.54 MB (70.99%) -- media │ ├──-0.41 MB (53.00%) ── libogg │ ├──-0.13 MB (16.91%) ── resources [-] │ └──-0.01 MB (01.08%) ── decoded/audio ├──-0.11 MB (13.82%) ── heap-unclassified ├──-0.09 MB (12.25%) -- window-objects/top(app://camera.gaiamobile.org/index.html, id=1) │ ├──-0.08 MB (010.00%) -- active/window(app://camera.gaiamobile.org/index.html) │ │ ├──-0.03 MB (04.42%) -- js-compartment(app://camera.gaiamobile.org/index.html) │ │ │ ├──-0.02 MB (02.54%) -- shapes │ │ │ │ ├──-0.01 MB (01.39%) -- gc-heap │ │ │ │ │ ├──-0.01 MB (01.44%) -- tree │ │ │ │ │ │ ├──-0.01 MB (01.38%) ── global-parented │ │ │ │ │ │ └──-0.00 MB (00.05%) ── non-global-parented │ │ │ │ │ └───0.00 MB (-0.05%) ++ (2 tiny) │ │ │ │ └──-0.01 MB (01.15%) -- malloc-heap │ │ │ │ ├──-0.01 MB (01.17%) ── tree-tables │ │ │ │ └───0.00 MB (-0.02%) ++ (2 tiny) │ │ │ ├──-0.01 MB (01.36%) ++ objects │ │ │ └──-0.00 MB (00.52%) ++ (3 tiny) │ │ ├──-0.03 MB (03.28%) -- dom │ │ │ ├──-0.02 MB (02.22%) ── element-nodes │ │ │ └──-0.01 MB (01.06%) ++ (2 tiny) │ │ └──-0.02 MB (02.30%) -- layout │ │ ├──-0.01 MB (01.24%) ++ (2 tiny) │ │ └──-0.01 MB (01.06%) ── rule-nodes │ └──-0.02 MB (02.26%) -- js-zone(0xNNN) │ ├──-0.01 MB (01.02%) ── type-pool │ ├──-0.01 MB (01.02%) ── unused-gc-things │ └──-0.00 MB (00.22%) ++ (3 tiny) ├──-0.02 MB (02.01%) -- heap-overhead │ ├──-0.01 MB (01.02%) ── bookkeeping │ └──-0.01 MB (00.99%) ── bin-unused ├──-0.01 MB (00.92%) -- js-non-window │ ├──-0.00 MB (00.52%) -- runtime │ │ ├──-0.00 MB (00.03%) -- script-sources │ │ │ ├──-0.01 MB (01.26%) -- source(scripts=92, <non-notable files>) │ │ │ │ ├──-0.01 MB (01.21%) ── misc [-] │ │ │ │ └──-0.00 MB (00.05%) ── uncompressed [-] │ │ │ └───0.01 MB (-1.23%) -- source(scripts=90, <non-notable files>) │ │ │ ├──0.01 MB (-1.19%) ── misc [+] │ │ │ └──0.00 MB (-0.04%) ── uncompressed [+] │ │ └──-0.00 MB (00.50%) ── script-data │ └──-0.00 MB (00.40%) ++ zones/zone(0xNNN) └──-0.00 MB (00.01%) ── atom-tables

I should clarify that the memory diff report in Comment 55 is comparing the v2.0 branch *WITHOUT* the patch to the v2.0 branch *WITH* the patch.

Comment on attachment 8456246 [details] pull-request (v2.0) Looking good!

Landed in v2.0: https://github.com/mozilla-b2g/gaia/commit/8a8622d58af2c24ef8df1c7fc7aa9cda50ac0085

Pull Request for master (v2.1) -- Carrying R+ forward

Landed on master: https://github.com/mozilla-b2g/gaia/commit/956b712ba7d4d1532e7099c6851db90cf3451130

No STR is present to create test case to address bug.