1070195 - Camera app kgsl memory growth during storing video thumbnails in 2.1

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Stability, defect, P1)

Description

[Tapas[:tkundu on #b2g/gaia/memshrink/gfx] (always NI me)]

+++ This bug was initially created as a clone of Bug #1069534 +++

STR:
1) Open camcorder and record 30 videos (each video duration is 5secs) .

We can see a linear kgsl growth within 5mins :

kgsl memory after opening camcorder app : 
$adb shell cat /sys/class/kgsl/kgsl/page_alloc
5488640

kgsl memory after recording one video:
adb shell cat /sys/class/kgsl/kgsl/page_alloc
23048192

kgsl memory after recording multiple videos:
adb shell cat /sys/class/kgsl/kgsl/page_alloc
34996224


We need a similar fix mentioned in bug 1069534 Comment 6

Comment 1

[Tapas[:tkundu on #b2g/gaia/memshrink/gfx] (always NI me)]

[Blocking Requested - why for this release]:

Comment 2

[Tapas[:tkundu on #b2g/gaia/memshrink/gfx] (always NI me)]

Putting NI on you as you have helped us in 1064126 on 2.0

Comment 3

[Tapas[:tkundu on #b2g/gaia/memshrink/gfx] (always NI me)]

I am also seeing that following leaks by trying STR from Comment 0 .

1) camera app VSS is increasing(it reached from 100MB to 200MB) 

2) Total ION memory has has jumped from 48MB to 139MB just after recording 1 video using Camera.

3) ION memory usage increases from 139MB to 155MB (Total ION 155MB http://pastebin.mozilla.org/6554078) if we keep repeating STR from Comment 0

4) kgsl memory growth mentioned in Comment 0.


There may be multiple issues which is causing this leak. But getting a fix from gaia camcorder code (see bug 1069534 Comment 6) may help us to see what is next step here.

Device used : msm8926 with 1GB memory.

Comment 4

[David Flanagan [:djf]]

Attachment: patch for master

I can't reproduce the bug on my 512mb Flame running master, but here's a patch that does the same thing we did for the Video app in bug 1069534

Comment 5

[David Flanagan [:djf]]

Attachment: patch vor v2.1

Tapas,

Does this fix the bug for you?

Comment 6

[Wilson Page [:wilsonpage]]

Comment on attachment 8492443 [details] [review]
patch for master

Looks fine, one tiny weeny nit.

Comment 7

[Tapas[:tkundu on #b2g/gaia/memshrink/gfx] (always NI me)]

(In reply to David Flanagan [:djf] from comment #4)
> Created attachment 8492443 [details] [review]
> patch for master
> 
> I can't reproduce the bug on my 512mb Flame running master, but here's a
> patch that does the same thing we did for the Video app in bug 1069534

Could you please re-base your patch for FFOS v2.1 ?

Comment 8

[Tapas[:tkundu on #b2g/gaia/memshrink/gfx] (always NI me)]

Please ignore my Comment 7. It was my misunderstand !

Comment 9

[David Flanagan [:djf]]

Review comment addressed and patches updated. It is purely an indentation issue, so it won't affect the outcome of any testing.

I'll wait to land this until I hear the results of Tapas's test run.

Comment 10

[Inder]

David -- we should get this fixed in 2.0 as well. Considering the change does not look risky and fixes a leak which customers will definitely run into.

Comment 11

[Tapas[:tkundu on #b2g/gaia/memshrink/gfx] (always NI me)]

Comment on attachment 8492447 [details] [review]
patch vor v2.1

Please land this fix for 2.1. We also want to see this is fixed in v2.0

Comment 12

[Tapas[:tkundu on #b2g/gaia/memshrink/gfx] (always NI me)]

We are not seeing any memory growth from #1, #2 , #3 and #4 of comment 3 after landing fix from  attachment 8492447 [details] [review] in v2.1 .

Comment 13

[David Flanagan [:djf]]

Landed on master: https://github.com/mozilla-b2g/gaia/commit/3563ddca35a52e7a533dcfc7996e74b78c6e36c8

Landed on v2.1: https://github.com/mozilla-b2g/gaia/commit/b3f9b97d16a1ab55f80239d63c1a85c3da3d39ad

Comment 14

[David Flanagan [:djf]]

Comment on attachment 8492443 [details] [review]
patch for master

Bhavana: this is a low-risk patch that CAF would like to have us uplift to 2.0. It mirrors a 2.0+ Video app patch (bug 1064126) that landed in 2.0 last week.

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): not a regression

[User impact] if declined: memory leaks when recording multiple videos with the camera app

[Testing completed]: verified by CAF

[Risk to taking this patch] (and alternatives if risky): not risky at all. All this patch does is set a canvas element size to zero after we are done using it so that the image memory it uses can be reclaimed more quickly. We landed a similar 2.0+ fix for the Video app a few weeks ago.

[String changes made]: none.

Comment 15

[David Flanagan [:djf]]

Uplifted to v2.0: https://github.com/mozilla-b2g/gaia/commit/0658006be8a00fdb5931ee15a0aa353a3ab231ba

Comment 16

[Kai-Zhen Li [:seinlin][:kli]]

Merged t v2.0m: https://github.com/mozilla-b2g/gaia/commit/0658006be8a00fdb5931ee15a0aa353a3ab231ba