1000448 - [Tarako] Please add the variable "GAIA_MEMORY_PROFILE=low" to the build script for all Tarako builds

Status: RESOLVED FIXED

(Release Engineering :: General, defect)

Description

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

As per the title, please add GAIA_MEMORY_PROFILE=low as a variable in the build script when bug 989290 lands.


From : apps/camera/js/config.js
The default value of these variables for the config.js for the camera app depends on the GAIA_MEMORY_PROFILE environment variable. Set
GAIA_MEMORY_PROFILE=low when building Gaia to get default
values suitable for low-memory devices.

Comment 1

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Hi David, I'm not sure if ni (needinfo) or cc works better for you... here's the bug you requested.

Comment 2

[Chris AtLee [:catlee]]

Any reason to not do this ahead of time?

You can set this in https://hg.mozilla.org/releases/mozilla-b2g28_v1_3t/file/4b8d599a5956/b2g/config/tarako/config.json#l21

Comment 3

[David Flanagan [:djf]]

(In reply to Chris AtLee [:catlee] from comment #2)
> Any reason to not do this ahead of time?
> 

GAIA_MEMORY_PROFILE is currently unused, so there is no reason it couldn't be set in config.json before the patch in 989290 lands.

> You can set this in
> https://hg.mozilla.org/releases/mozilla-b2g28_v1_3t/file/4b8d599a5956/b2g/
> config/tarako/config.json#l21

I actually don't know how to prepare a gecko patch against anything branch other than m-c, so I'm going to leave that to someone from release engineering.

Comment 4

[David Flanagan [:djf]]

Marking this as a blocker because the 1.3T nightly builds won't work right unless this is fixed.

Needinfo Chris: do you know someone who can work on this?  As you point out in comment #2, it is just a one-line change.

Comment 6

[James Zhang (Spreadtrum)]

Hi David,

   I have landed camera.json on my side.


commit b0c1ab607224b96740cb1b9f51fe0344194ab0cd
Author: james.zhang <[e-mail]>
Date:   Sat Apr 26 13:09:39 2014 +0800

    Bug#269156 mozilla Bug 1000448 - [Tarako] Please add the variable "GAIA_MEMORY_PROFILE=low" to the build script for all Tarako builds
    
    [bug number  ]
    [root cause  ]
    [changes     ]
    [side effects]
    [self test   ] yes
    [reviewers   ]
    
    Change-Id: I7dd39ced5eb7f7a5a62da2b4f61fc2592826d688

diff --git a/sp6821a_gonk/camera.json b/sp6821a_gonk/camera.json
index 5edd2c5..dd690e1 100644
--- a/sp6821a_gonk/camera.json
+++ b/sp6821a_gonk/camera.json
@@ -1,4 +1,7 @@
 {
    "maxImagePixelSize": 2097152,
-   "maxSnapshotPixelSize": 2097152
-}
\ No newline at end of file
+   "maxSnapshotPixelSize": 1920000,
+   "maxGifImageFileSize": 1048576,
+   "maxPickPixelSize": 480000,
+   "maxEditPixelSize": 480000
+}

Comment 7

[Tim Guan-tin Chien [:timdream] (please needinfo)]

I don't understand why we need this bug as 1.3T+.

We already have POVB bug 998228 for Tarako builds, which essentially do the same thing on camera/gallery config. Unless David have some extra tweak he like to do based on this variable, we shouldn't need to set this for Tarako build.

Please also noted that B2G build script does not pass every variable to Gaia build script, so adding this switch in the build might not be as simple as bug 998228 either.

Comment 8

[Tim Guan-tin Chien [:timdream] (please needinfo)]

The summary of this bug and comment 0 has nothing to do with changes made in comment 6. Please keep all record centralized and update whenever possible.

Comment 9

[Tim Guan-tin Chien [:timdream] (please needinfo)]

I just realized the information was sitting in my inbox yesterday (Friday night ...)

So this bug has nothing to do with |GAIA_MEMORY_PROFILE=low| ... it's about adding what was added in comment 6. And again, |GAIA_MEMORY_PROFILE=low| is not needed if comment 6 and bug 998228 are in-placed for Tarako.

I am updating summary here. If that's not correct please revert my change.

(Keeping :djf needinfo)

Comment 10

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Should we close this bug since comment 6 is landed on the partner repo?

Comment 11

[David Flanagan [:djf]]

Tim,

This bug is for release engineering so that our nightly Tarako builds work. Until GAIA_MEMORY_PROFILE=low is set, my patches rotate images without downsampling them and we get more OOMs.  This is now a smoketest blocker, actually.

So I'm setting the bug description back.

Comment 12

[Tim Guan-tin Chien [:timdream] (please needinfo)]

David, I am confused, pvtbuild picks up the same device config (camera.json) from partner repo. I don't think we need to ask releng to set |GAIA_MEMORY_PROFILE=low| specifically and separately.

(Unless what being told is wrong.)

Comment 13

[Joe Cheng [:jcheng] (please needinfo)]

ni? djf

Comment 14

[Natalya Kot [:nkot]]

This bug was called out as a smoketest blocker due to inability to add camera taken picture to a contact (see dup bug 1001675). On today's build we no longer reproduce this issue - picture can be added successfully, no OOM.

I am not too sure if we can close this bug now.

Tarako 1.3t
BuildID: 20140428014001
Gaia: 8895b180ed636069473703d0e7b73086989601ce
Gecko: 7caf4b5abfce
Version: 28.1
sp6821a_gonk4.0_user.pac

Comment 15

[David Flanagan [:djf]]

Thanks for the clarification, Tim. The creation of builds is all a mystery to me. I assumed that we were making our own builds that did not depend on the partner repo.

Given Natalya's comment, I think we can close this.

Comment 16

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

I'm a little confused about the situation.  Does this mean we have duplication of code?
https://bugzilla.mozilla.org/show_bug.cgi?id=1000448#c6

One at the gonk and one at the gaia?  Should I test to see if I can remove the gaia camera.json via not using the variable?

Comment 17

[David Flanagan [:djf]]

clearing the needinfo for me since I understand nothing about how builds are created.

Comment 18

[John Hammink]

Natalya - do you guys know if you are doing a full flash on tarako for daily smoketest or just adding gecko/gaia?  This is important because I've just updated gaia/gecko and I'm definitely getting a hang when using camera to update smoketests.

Comment 19

[Natalya Kot [:nkot]]

(In reply to John Hammink from comment #18)
> Natalya - do you guys know if you are doing a full flash on tarako for daily
> smoketest or just adding gecko/gaia?  This is important because I've just
> updated gaia/gecko and I'm definitely getting a hang when using camera to
> update smoketests.

We flash entire tarako.zip from pvt builds, that includes gaia/gecko/gonk in the form of image files.

Comment 20

[John Hammink]

what I'm seeing is that on gecko/gaia builds only

GAIA_MEMORY_PROFILE=low is not set.  When I flash only gecko/gaia I am getting,  a camera preview freeze when trying to add a picture (from camera) to contacts.

Gaia      8895b180ed636069473703d0e7b73086989601ce
Gecko     https://hg.mozilla.org/releases/mozilla-b2g28_v1_3t/rev/7caf4b5abfce
BuildID   20140428014001

However, when I do a full flash, the problem goes away.  

This suggest that full builds use the camera.json file from the spreadtrum tree, but gecko/gaia builds still need GAIA_MEMORY_PROFILE=low set.  As djf suggested, let's reopen this.

Comment 21

[Chris AtLee [:catlee]]

Attachment: lowmem-mozilla-b2g28_v1_3t.patch

Comment 22

[Tim Guan-tin Chien [:timdream] (please needinfo)]

(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) (please ni?) from comment #12)
> David, I am confused, pvtbuild picks up the same device config (camera.json)
> from partner repo. I don't think we need to ask releng to set
> |GAIA_MEMORY_PROFILE=low| specifically and separately.

This is what I am told and this is all I know.

I am also told there were some kernel/gonk improvement on the platform side since Friday, so if we no longer reproduce this bug, it is most likely due to that change instead of a pref change in Gaia.

(In reply to John Hammink from comment #20)
> what I'm seeing is that on gecko/gaia builds only
> 
> GAIA_MEMORY_PROFILE=low is not set.  When I flash only gecko/gaia I am
> getting,  a camera preview freeze when trying to add a picture (from camera)
> to contacts.
> 
> Gaia      8895b180ed636069473703d0e7b73086989601ce
> Gecko    
> https://hg.mozilla.org/releases/mozilla-b2g28_v1_3t/rev/7caf4b5abfce
> BuildID   20140428014001
> 
> However, when I do a full flash, the problem goes away.  
> 
> This suggest that full builds use the camera.json file from the spreadtrum
> tree, but gecko/gaia builds still need GAIA_MEMORY_PROFILE=low set.  As djf
> suggested, let's reopen this.

You are most likely did not pick up that Gonk change on Friday. Please talk to your team to make sure everything is flashing and testing on the same build.

Comment 23

[Tim Guan-tin Chien [:timdream] (please needinfo)]

s/everything/everyone/

Comment 24

[thomas tsai]

Since partner has very frequent update to kernel and hal and setttings, it's necessary to do full update.
Full update means you have to update boot.img, system.img, userdata.img daily. Only gecko and gaia is not enough to make tarako work.

Comment 25

[thomas tsai]

The detail update procedure, please refer to https://intranet.mozilla.org/B2G_Team/Tarako

Comment 26

[Tim Guan-tin Chien [:timdream] (please needinfo)]

(In reply to Chris AtLee [:catlee] from comment #21)
> Created attachment 8414145 [details] [diff] [review]
> lowmem-mozilla-b2g28_v1_3t.patch

I've also realized we can NEVER make gecko/gaia builds pick up ALL Gaia customization from partner (unless we want to build Gaia against partner repo), so any test from this build can not be taken from granted. (We have switch like GAIA_MEMORY_PROFILE in this case only)

Comment 27

[James Zhang (Spreadtrum)]

(In reply to thomas tsai from comment #25)
> The detail update procedure, please refer to
> https://intranet.mozilla.org/B2G_Team/Tarako

Thomas, let's discuss pvt build.

Comment 28

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

We have gecko/gaia flashing as well as a full build flashing.

This would allow the gecko/gaia flash to work if we have this patch.

I believe we should be full flashing for other reasons.  Having said that this patch would allow us to flash this gecko/gaia onto any other device running the same android sdk as tarako.

I'm not sure if we'll ever do this though.  We should be full flashing for other reasons that Thomas has mentioned.

Comment 29

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

The only other thing I can think of is OTA, which we only do gecko/gaia flashing, now that I think about it.  If they don't have the gonk level changes, they will run into the same issue that John ran into...

Comment 30

[Chris AtLee [:catlee]]

Comment on attachment 8414145 [details] [diff] [review]
lowmem-mozilla-b2g28_v1_3t.patch

https://hg.mozilla.org/releases/mozilla-b2g28_v1_3t/rev/df4ec76ceccc

Comment 31

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Why is this bug is kept opened?

Comment 32

[James Zhang (Spreadtrum)]

What can I do on my side?

Comment 33

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

based on comment 31, I think our releng fixed this issue? and this could be closed.  Ni?catlee to confirm.

Comment 34

[Chris AtLee [:catlee]]

this is fixed from my POV.

Comment 35

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Thanks, catlee!