1237568 - [TV] Browser app can be killed by OOM with big history or bookmarks stores

Status: RESOLVED INVALID

(Firefox OS Graveyard :: Gaia::TV::Browser, defect, P1)

Description

[Fernando Jiménez Moreno [:ferjm]]

We are currently saving in memory the whole list of history and bookmark records [1][2]. This can cause the browser app to be killed if any of these lists or records are big enough.

[1] https://mxr.mozilla.org/gaia/source/tv_apps/browser/js/bookmarkStore.js#23
[2] https://mxr.mozilla.org/gaia/source/tv_apps/browser/js/historyStore.js#14

Comment 1

[Fernando Jiménez Moreno [:ferjm]]

This issue won't let us land bug 1232276 for the TV to sync the whole list of bookmarks or history :( Check bug 1237570

Comment 2

[yifan [:yifan][:yliao]]

Thank you for figuring it out! Dan could you please work on this?

Comment 3

[Dan Huang[:danhuang]]

Got it! I am working on this.

Comment 4

[Dan Huang[:danhuang]]

Hi Fernando, I had used webIDE to snapshot browser's memory usage between open sync history list and close the list. It seems the memory usage didn't increase continuously after closing the list. So could you tell us how do you get the OOM in browser? Thanks.

Comment 5

[Fernando Jiménez Moreno [:ferjm]]

Hello Dan,

I didn't get any OOM because I can't test this on the real TV. On B2G Desktop we are running with a single process and we have no OOM killer. So unfortunately it's currently not possible to test this.

However, looking at the code, it gave me the impression that the OOM situation is possible. With the current setting you probably won't see a significant peak of memory given that we limit the number of records that we fetch from the server. But with no control over this, like we were adding on bug 1232276, we may end up fetching a significantly big number of records. For example, I have around 15K history records on my account. If I understood the code properly, we may end up loading in memory the whole list of history if the user scrolls down through the history list. Is this correct?

If my assumption above is not correct, then we can safely close this bug :)

Comment 6

[Dan Huang[:danhuang]]

Hi Fernando, 

The browser will not load all history data in the memory as user scroll down the list. When user first open the list, the browser load only one day's history and save in memory (this.cache) for display. As user scroll down to the end of list, browser would delete the data in this.cache, then load the next day's data and save in this.cache. So this.cache would keep only one day's history data as maximum.

Also in HTML Dom rendering, when user scrolling the list, we use only 21 list element as maximum to limit the memory consumption.

For more safety,  Sean and I will check with performance team on Monday to see if they have any good way can check or verify javascript's memory usage.

Comment 7

[Sean Lee [:seanlee][:weilonge]]

Hello all,

While we are discussing the memory consumption issue, there are some requirements for the memory profiling tool in my mind:
1. Peak value during a specific time.
2. Overall report to summarize the memory usage for Gaia.
3. Object allocation statistics corresponded to a script/app. 

We got a feedback from Performance team that we can use Memory Profiler [1][2] add-on to investigate the memory consumption for a specific time slot. 
There are some steps to install the add-on:
a. Download Mulet by 'make mulet' or other way. 
b. In about:config, type and search this configure "xpinstall.signatures.required" and set it to false (double click "value" field to change value)
c. Download [3] and drag the add-on file to Mulet, and then install it.

Now you can see there is 'Memory Profiler' tab in developer tools.
Click start/stop to record the memory usage.

[1] https://wiki.mozilla.org/MemoryProfiler
[2] https://developer.mozilla.org/en-US/docs/Mozilla/Performance/Memory_Profiler
[3] https://www.dropbox.com/s/3ynu5f6hk7uy91y/memory-profiler-ui-0.1.xpi?dl=0

Comment 8

[Sean Lee [:seanlee][:weilonge]]

Attachment: Screen Shot for Memory Profiler

Comment 9

[Sean Lee [:seanlee][:weilonge]]

In attachment 8706747 [details], self peak and total peak can tell the information about peak memory consumption. Kan-ru is the project owner. I will discuss detail with him (he's in business trip now).

Comment 10

[Dan Huang[:danhuang]]

Attachment: historyStore_getByIndex.png

Comment 11

[Dan Huang[:danhuang]]

Attachment: historyStore_getCache.png

Comment 12

[Dan Huang[:danhuang]]

After confirm with Kan-ru, we can trace the OOM issue by two columns: 'Self Size' and 'Total Size'. Self size means the memory size didn't be released by the specific function. And the total size means the total memory size didn't be released by the specific javascript file.
I create a test case which will sync 10000 history records and scroll down the whole history list repeatly for 10 times. The value of 'Self Size' and 'Total Size' in historyStore.js is 0(attachment 8710885 [details],  attachment 8710886 [details] ).

So I think maybe there is no OOM issue in historyStore and bookmarkStore.

Comment 13

[Fernando Jiménez Moreno [:ferjm]]

That's great news Dan. Thanks for checking it!