1045503 - [tarako][contacts] we should reduce the use of storage by contacts app

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Contacts, defect)

Description

[Wayne Chang [:wchang]]

As the tarako device has a small internal storage partition we should reduce the storage uses by our core apps, contacts especially seem to be using quite a bit.

Comment 1

[Wayne Chang [:wchang]]

ni? KaiZhen as he has some stats on the storage used by importing contacts.

Steve, can you also share your analysis here?

Thanks

Comment 2

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

After imported contact from facebook and gmail, I noticed that it consumes about 31MB from /data/local.

$ du -sh local/storage/persistent/*
192K local/storage/persistent/1003+f+app+++video.gaiamobile.org
48K  local/storage/persistent/1005+f+app+++wappush.gaiamobile.org
1.7M local/storage/persistent/1006+f+app+++gallery.gaiamobile.org
560K local/storage/persistent/1008+f+app+++homescreen.gaiamobile.org
48K  local/storage/persistent/1010+f+app+++costcontrol.gaiamobile.org
48K  local/storage/persistent/1011+f+app+++calendar.gaiamobile.org
48K  local/storage/persistent/1012+f+app+++callscreen.gaiamobile.org
48K  local/storage/persistent/1013+f+app+++camera.gaiamobile.org
192K local/storage/persistent/1014+f+app+++email.gaiamobile.org
88K  local/storage/persistent/1015+f+app+++communications.gaiamobile.org
11M  local/storage/persistent/1016+f+app+++music.gaiamobile.org
236K local/storage/persistent/1017+f+app+++browser.gaiamobile.org
88K  local/storage/persistent/1018+f+app+++clock.gaiamobile.org
48K  local/storage/persistent/1019+f+app+++sms.gaiamobile.org
48K  local/storage/persistent/1022+f+app+++system.gaiamobile.org
88K  local/storage/persistent/1024+f+app+++marketplace.firefox.com
84K  local/storage/persistent/1031+f+app+++{f7f75813-8f7a-4a3f-87b0-093662a3dfa9}
31M  local/storage/persistent/chrome

Comment 3

[Julien Wajsberg [:julienw]]

Note that "local/storage/persistent/chrome" contains several DB, you should go inside the directory and look at the sizes of the various DB.

Comment 4

[Steve Chung [:steveck]]

Wayne and Kai-Zhen, may I know the test environment for the contact app? How many contacts did you added/imported for the testing?

Comment 5

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

Before import contacts, local/storage/persistent/chrome consumed only about 500K. After imported contact from facebook and gmail, it is increased to about 31MB. It is about 600 from facebook and 200 from gmail. Most from gmail is without picture.

$ du -sh local/storage/persistent/chrome/idb/*
48K	local/storage/persistent/chrome/idb/226660312ssm.sqlite
0	local/storage/persistent/chrome/idb/226660312ssm.sqlite-journal
172K	local/storage/persistent/chrome/idb/2588645841ssegtnti
100K	local/storage/persistent/chrome/idb/2588645841ssegtnti.sqlite
0	local/storage/persistent/chrome/idb/2588645841ssegtnti.sqlite-journal
80K	local/storage/persistent/chrome/idb/3104902905ascetiitvi.sqlite
0	local/storage/persistent/chrome/idb/3104902905ascetiitvi.sqlite-journal
40K	local/storage/persistent/chrome/idb/3249156127nsetta_ts.sqlite
0	local/storage/persistent/chrome/idb/3249156127nsetta_ts.sqlite-journal
48K	local/storage/persistent/chrome/idb/3406066227csotncta
2.6M	local/storage/persistent/chrome/idb/3406066227csotncta.sqlite
0	local/storage/persistent/chrome/idb/3406066227csotncta.sqlite-journal
44K	local/storage/persistent/chrome/idb/4045445992aslmar.sqlite
0	local/storage/persistent/chrome/idb/4045445992aslmar.sqlite-journal
27M	local/storage/persistent/chrome/idb/4272465303Gpapiaab_eFwa.ctesbeo
1.2M	local/storage/persistent/chrome/idb/4272465303Gpapiaab_eFwa.ctesbeo.sqlite
0	local/storage/persistent/chrome/idb/4272465303Gpapiaab_eFwa.ctesbeo.sqlite-journal
40K	local/storage/persistent/chrome/idb/846562544phus.sqlite
0	local/storage/persistent/chrome/idb/846562544phus.sqlite-journal

Comment 6

[Steve Chung [:steveck]]

Hey Francisco, I've discovered that contact photo imported from facebook was save as png type. It seems image downloaded from server is PNG by default and it will take much more memory then jpg by default. Maybe we can convert to jpg before we saved to DB, wdyt?

Comment 7

[Wayne Chang [:wchang]]

Echoing Steve - not sure if services like facebook sync provides jpg download also? If so we can take that option easier...

Comment 8

[Julien Wajsberg [:julienw]]

48K	local/storage/persistent/chrome/idb/3406066227csotncta
2.6M	local/storage/persistent/chrome/idb/3406066227csotncta.sqlite
=> clearly, the contacts database is not too big (but probably because there are no pictures too)

27M	local/storage/persistent/chrome/idb/4272465303Gpapiaab_eFwa.ctesbeo
1.2M	local/storage/persistent/chrome/idb/4272465303Gpapiaab_eFwa.ctesbeo.sqlite
=> less data in the sqlite than for contacts (probably because there are less indices)
=> a lot more data in the attachments (pictures)

Using jpg is likely a good idea :)

Comment 9

[Wayne Chang [:wchang]]

Flagging here for traction. If we can get this through the device would be a lot more usable for the end users.

Comment 10

[Keven Kuo [:kkuo]]

Hi! Tim,

Could someone of your team can take this case? Thanks

--
Keven

Comment 11

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

Steve will be available as fallback team if everyone else is working on more urgent stuff.

Comment 12

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

BTW, Facebook does not indicate the format of the image they return nor there is a parameter available for explicitly specifying the format.

https://developers.facebook.com/docs/graph-api/reference/v2.0/user/picture

From my experiment Facebook always returns JPEG (try the URL below for example).

http://graph.facebook.com/mozilla/picture

I don't know why we are getting PNG in the storage in this function since we are indeed using the same API.
https://github.com/mozilla-b2g/gaia/blob/master/shared/js/contacts/import/facebook/fb_query.js#L94-L139

Comment 13

[Julien Wajsberg [:julienw]]

Maybe we're using canvas' toBlob or toDataURL which uses png by default.

Comment 14

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

(In reply to Julien Wajsberg [:julienw] from comment #13)
> Maybe we're using canvas' toBlob or toDataURL which uses png by default.

I did grep these two function names and didn't find them :'(

Comment 15

[Luke Chang [:lchang] (inactive)]

(In reply to Julien Wajsberg [:julienw] from comment #13)
> Maybe we're using canvas' toBlob or toDataURL which uses png by default.

I found that we call "toBlob" here: https://github.com/mozilla-b2g/gaia/blob/v1.3t/apps/communications/contacts/js/utilities/image_thumbnail.js#L46

Comment 16

[Steve Chung [:steveck]]

Hey Francisco, we found toBlob return png by default, so we must force the mime type for conversion.
Please also down scale the image quality in image_square.js because image download from facebook seems use lower quality then toBlob default(0.95), thanks!

Comment 17

[Francisco Jordano [:arcturus] [:francisco]]

Hei Steve,

thanks for diving into to this.

Will try to provide a patch ASAP.

Comment 18

[Francisco Jordano [:arcturus] [:francisco]]

Attachment: Pointer to PR 22311

Comment 19

[Luke Chang [:lchang] (inactive)]

Attachment: Pull Request for v1.3t

Sorry to jump in.

Due to the urgency of Tarako, I'm helping to uplift Francisco's patch to v1.3t so QA can be involved ASAP. I'll back it out if by any chance the original patch get r-.

In addition, I also decreased the quality of the "jpeg" compression in the patch since we have to reduce the storage consumption on Tarako as much as possible.

Tim, could you please help to review this Tarako-only patch. Thanks.

Comment 20

[Luke Chang [:lchang] (inactive)]

Set qawanted for attachment 8465184 [details] [review]. Thanks.

Comment 21

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

Comment on attachment 8465184 [details] [review]
Pull Request for v1.3t

rubberstamping -- this does not affect review of patch on master.

Please make a note here on the quality parameter used (0.65).

Comment 22

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

Before jpg is used for import contact from facebook occupied about 28MB.

After jpg is use, I can see the application storage used by contact is decreased.

So I also suggest to use 0.65 on master, as the image quality is also acceptable and it use less storage. The image quality of 0.35 is blurry and the storage is not saved much than 0.65.

--
0.95 18M   local/storage/persistent/chrome

0.65 12M   local/storage/persistent/chrome
     8.6M  local/storage/persistent/chrome/idb/4272465303Gpapiaab_eFwa.ctesbeo
     1.3M  local/storage/persistent/chrome/idb/4272465303Gpapiaab_eFwa.ctesbeo.sqlite

0.35 12M   local/storage/persistent/chrome
     8.0M  local/storage/persistent/chrome/idb/4272465303Gpapiaab_eFwa.ctesbeo
     1.2M  local/storage/persistent/chrome/idb/4272465303Gpapiaab_eFwa.ctesbeo.sqlite

Comment 23

[Brian Huang [:brianhuang][:brhuang]]

ni? Mike for him to verify with the patch provided by Luke.

Comment 24

[Evelyn Hung]

Thanks Kai-zhen for pasting testing result of previous patch. 
We will let QA test the patch as well, and land 1.3T patch if there is no problem found.

Comment 25

[James Zhang (Spreadtrum)]

1.3t
981af92d04f5c1bf0423ea61545cc87702bae0ba

Comment 26

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

I think we also need this on master.

Comment 27

[Jose Manuel Cantera]

Comment on attachment 8464682 [details] [review]
Pointer to PR 22311

trivial change. r+ from my side and I left to the implementor the decision of what quality params to be used

Comment 28

[Mike Lien[:mlien]]

verify patch with 350 FB contacts + 100 outlook contacts
reduced usage of storage near 15MB

original:
40K	data/local/storage/persistent/chrome/idb/226660312ssm.sqlite
0	data/local/storage/persistent/chrome/idb/226660312ssm.sqlite-journal
172K	data/local/storage/persistent/chrome/idb/2588645841ssegtnti
96K	data/local/storage/persistent/chrome/idb/2588645841ssegtnti.sqlite
0	data/local/storage/persistent/chrome/idb/2588645841ssegtnti.sqlite-journal
72K	data/local/storage/persistent/chrome/idb/3104902905ascetiitvi.sqlite
0	data/local/storage/persistent/chrome/idb/3104902905ascetiitvi.sqlite-journal
40K	data/local/storage/persistent/chrome/idb/3249156127nsetta_ts.sqlite
0	data/local/storage/persistent/chrome/idb/3249156127nsetta_ts.sqlite-journal
3.8M	data/local/storage/persistent/chrome/idb/3406066227csotncta
1.5M	data/local/storage/persistent/chrome/idb/3406066227csotncta.sqlite
0	data/local/storage/persistent/chrome/idb/3406066227csotncta.sqlite-journal
40K	data/local/storage/persistent/chrome/idb/4045445992aslmar.sqlite
0	data/local/storage/persistent/chrome/idb/4045445992aslmar.sqlite-journal
17M	data/local/storage/persistent/chrome/idb/4272465303Gpapiaab_eFwa.ctesbeo
648K	data/local/storage/persistent/chrome/idb/4272465303Gpapiaab_eFwa.ctesbeo.sqlite
0	data/local/storage/persistent/chrome/idb/4272465303Gpapiaab_eFwa.ctesbeo.sqlite-journal
40K	data/local/storage/persistent/chrome/idb/846562544phus.sqlite
0	data/local/storage/persistent/chrome/idb/846562544phus.sqlite-journal

with patch:
40K	datalocal/storage/persistent/chrome/idb/226660312ssm.sqlite
0	datalocal/storage/persistent/chrome/idb/226660312ssm.sqlite-journal
172K	datalocal/storage/persistent/chrome/idb/2588645841ssegtnti
96K	datalocal/storage/persistent/chrome/idb/2588645841ssegtnti.sqlite
0	datalocal/storage/persistent/chrome/idb/2588645841ssegtnti.sqlite-journal
76K	datalocal/storage/persistent/chrome/idb/3104902905ascetiitvi.sqlite
0	datalocal/storage/persistent/chrome/idb/3104902905ascetiitvi.sqlite-journal
40K	datalocal/storage/persistent/chrome/idb/3249156127nsetta_ts.sqlite
0	datalocal/storage/persistent/chrome/idb/3249156127nsetta_ts.sqlite-journal
660K	datalocal/storage/persistent/chrome/idb/3406066227csotncta
1.5M	datalocal/storage/persistent/chrome/idb/3406066227csotncta.sqlite
0	datalocal/storage/persistent/chrome/idb/3406066227csotncta.sqlite-journal
40K	datalocal/storage/persistent/chrome/idb/4045445992aslmar.sqlite
0	datalocal/storage/persistent/chrome/idb/4045445992aslmar.sqlite-journal
4.6M	datalocal/storage/persistent/chrome/idb/4272465303Gpapiaab_eFwa.ctesbeo
656K	datalocal/storage/persistent/chrome/idb/4272465303Gpapiaab_eFwa.ctesbeo.sqlite
0	datalocal/storage/persistent/chrome/idb/4272465303Gpapiaab_eFwa.ctesbeo.sqlite-journal
40K	datalocal/storage/persistent/chrome/idb/846562544phus.sqlite
0	datalocal/storage/persistent/chrome/idb/846562544phus.sqlite-journal

Comment 29

[Francisco Jordano [:arcturus] [:francisco]]

Landed on master:

https://github.com/mozilla-b2g/gaia/commit/464599a069d9fed208dec90692c66e91ffc070ea

Thanks everyone involved!

Comment 30

[Julien Wajsberg [:julienw]]

Francisco, the improvement is so big for a so safe and simple change, that I think we should uplift to v2.0. I let you request an approval?

Comment 31

[Francisco Jordano [:arcturus] [:francisco]]

Hi Julien,

You are definitely right, will request the uplift with current quality 0.95.

Thanks for the remind!

Comment 32

[Francisco Jordano [:arcturus] [:francisco]]

Comment on attachment 8464682 [details] [review]
Pointer to PR 22311

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
[Bug caused by] 
We are storing png images instead of compressed jpeg ones
[User impact]
We are consuming too much internal memory and that affects the space available for the user.
[Testing completed]:
Tested on master, also this patch has been uplift to 1.3T
[Risk to taking this patch] (and alternatives if risky):
Minimal risk, we are just requesting the platform a jpeg format instead of the png one
[String changes made]:

Comment 33

[bhavana bajaj [:bajaj]]

Comment on attachment 8464682 [details] [review]
Pointer to PR 22311

Given the super low risk and the internal memory wins here, we'll take this on 2.0..Any fallouts/regressions this causes, please backout immediately.

Comment 34

[Ryan VanderMeulen [:RyanVM]]

v2.0: https://github.com/mozilla-b2g/gaia/commit/b3675c8e55e2b784b5e5dbb23df6bd281f85732a

Worth considering for Dolphin on v1.4 as well?

Comment 35

[Francisco Jordano [:arcturus] [:francisco]]

(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #34)
> v2.0:
> https://github.com/mozilla-b2g/gaia/commit/
> b3675c8e55e2b784b5e5dbb23df6bd281f85732a
> 
> Worth considering for Dolphin on v1.4 as well?

Definitely, should I ask for the flags?

Comment 36

[Julien Wajsberg [:julienw]]

[Blocking Requested - why for this release]:

Comment 37

[Wayne Chang [:wchang]]

Agree that this is needed for 1.4. Instead of flagging we should request for approval.

Francisco, 
Can you request for 1.4 approval on the patch if it'll land directly?

Thanks

Comment 38

[Francisco Jordano [:arcturus] [:francisco]]

Comment on attachment 8465184 [details] [review]
Pull Request for v1.3t

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
[Bug caused by]
Not a bug, but a feature for using less space on images downloaded.
[User impact] if declined:
A lot of usage in internal memory, not allowing the user to use the phone for other stuff like photos, music, etc.
[Testing completed]:
Yes, in versions 1.3 and 2.0
[Risk to taking this patch] (and alternatives if risky):
Pretty low risk, since we are just saying gecko to compress the images when we save them.
[String changes made]:

Comment 39

[Ryan VanderMeulen [:RyanVM]]

v1.4: https://github.com/mozilla-b2g/gaia/commit/4d5451815fbab514fbdfed4268d9f683e4e05bb4