1142845 - [Contacts][dolphin][FFOS7715 v2.1s][performance] Dolphin takes a longer time to export 1000 contacts to SD card

Status: RESOLVED INCOMPLETE

(Core Graveyard :: DOM: Contacts, defect)

Description

[Shiwei Zhang]

*** Build Information
Gaia-Rev        37c364a4b055ca8ae7bb08dcd9ee6373771c78fd
Gecko-Rev       dc1154beadf9845a8783c1ccb1ed75c101ea8b1e
Device-Name     dolphin


*** Expected Results
It takes shorter time to finish the operation, not more than 7.554s.

*** Actual Results
It takes a long time to finish the operation, average time is 9.632s.

Comment 1

[Shiwei Zhang]

we have recorded the time cost in interface and the result might help.

(1)In contacts/js/views/settings.js from onSelectedContacts in doExport() to new ContactsExporter(strategy) in promise.onsuccess()
cost 3s 450ms

(2)In contacts/js/views/settings.js from exporter.init() to exporter.start() called
cost 2s 260ms

(3)from exporter.start() in contacts/js/views/settings.js
to _start() in contacts/js/export/contacts_exporter.js
cost 10ms

(4)In contacts/js/export/sd.js, from ContactToVcard to getStorage()
cost 3s 400ms

(5)in contacts/js/export/sd.js, from getStorage() to request = storage.addNamed()
cost 100ms

(6)contacts/js/export/sd.js from request = storage.addNamed() to request.onsuccess
cost 160ms

Comment 2

[Julien Wajsberg [:julienw]]

Is it a Core / DOM: Contacts or a Firefox OS / Gaia::Contacts issue ?

What do you think Francisco ?

Comment 3

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

It looks to me as a Core/Dom: Contacts issue, also it will be interesting to know which version of gecko and gaia was being used.

Also, dolphin by itself is slow to accessing SD card.

Comment 4

[Shiwei Zhang]

(In reply to Francisco Jordano [:arcturus] [:francisco] from comment #3)
Hi Francisco,
  This is the version info,
  gaia remote="mozillaorg" revision="88084bc7ef5ba6627dd09c774ef2f7fa96cbed71" upstream="v2.1"
  gecko remote="mozillaorg" revision="c4f38a3b2cd7460239b704ad71e3d4968369de95" upstream="v2.1"

  And I think accessing SD card is not so slow, according to (5) (6) in comment1, it takes only 260ms
to getStorage and add the vcf file.
  I think createSelectPromise() in contacts/js/views/list.js is slow in (1), it is the first issue we can improve, and the main work of this function is navigator.mozContacts.find({}) and _selected.push().

Comment 5

[Vance Chen [:vchen][skype:vance.lucida][[e-mail]]]

Hi Francisco, Per Comment#4, is there any information you need from partner to help you to further investigate this problem?

Thanks

Comment 6

[Steven Yang [:styang]]

this is a partner blocker.

Comment 7

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

Hi all,

we could take a look to this after 2.2 FC.

Fernando do you think we can improve this?

Comment 8

[Vance Chen [:vchen][skype:vance.lucida][[e-mail]]]

Hi Fernando, since this is a blocking issue for our partner, do you think when you will be available to work on this?

Thanks

Comment 9

[Fernando Jiménez Moreno [:ferjm]]

I took a look at this one yesterday, but it doesn't seem like an easy fix. I'll keep working on it next week.

Comment 10

[Fernando Jiménez Moreno [:ferjm]]

I don't get the numbers that you mention in the description.

Exporting 1000 contacts takes ~30 seconds on a Flame (I don't have a Dolphin to test). You are saying in the description that it takes "9.632s". If the "s" stands for seconds, that time  is way better than the ones that I am getting on a Flame, which is surprising. Could you clarify which are the timings that you are getting and which are the expected times, please?

Also, it seems that bug 1103446 was not uplifted to 2.1. Could you try applying it on your tree. That patch improves mozContacts.find and I don't think we can improve it much more.

Thanks!

Comment 11

[Fernando Jiménez Moreno [:ferjm]]

Sorry, where I said ~30 seconds, I meant ~20 seconds.

Comment 12

[Fernando Jiménez Moreno [:ferjm]]

I tried avoiding the creation of a new file for every batch of contacts using the deviceStorage.getEditable() function, but I hit bug 859696.
The two slow points that I found are:
1. getting the list of contacts to export (mozContacts.find), which as I mentioned in comment 10 was improved in bug 1103446.
2. the need of creating a new file per each batch of contacts. We won't be able to fix that until bug 859696 is fixed.

Comment 13

[Fernando Jiménez Moreno [:ferjm]]

Using deviceStorage.appendNamed instead of deviceStorage.addNamed we avoid creating a new file per each batch of contacts and we save ~5 seconds exporting 1000 contacts (now we are taking ~15 seconds to export 1K contacts). It also saves some memory, but I still need to measure it.

Comment 14

[Shiwei Zhang]

Attachment: 1000.vcf

(In reply to Fernando Jiménez Moreno [:ferjm] from comment #10)
> I don't get the numbers that you mention in the description.
> 
> Exporting 1000 contacts takes ~30 seconds on a Flame (I don't have a Dolphin
> to test). You are saying in the description that it takes "9.632s". If the
> "s" stands for seconds, that time  is way better than the ones that I am
> getting on a Flame, which is surprising. Could you clarify which are the
> timings that you are getting and which are the expected times, please?

Thank you Fernando, maybe you can have a try with the vcf in attachment, of course, “s” is seconds, Exporting 1000 contacts takes no more than 8 seconds in my tests on a Flame.

After 1000 contacts were selected, we started timing by clicking the "Export" button, if we found a box prompting "1000/1000 contact exported" then stop timing.

> Also, it seems that bug 1103446 was not uplifted to 2.1. Could you try
> applying it on your tree. That patch improves mozContacts.find and I don't
> think we can improve it much more.
> 
> Thanks!

I will have a try as soon as possible.
Thanks.

Comment 15

[Shiwei Zhang]

Hi Fernando,
 I applyed this patch in gecko on v2.1s
https://hg.mozilla.org/mozilla-central/rev/aa2ab2b3f01b

as a result, things are going well, 5～6s are saved in exporting 1000 contacts to SD card.

I'll examine how it works if I have time.

Thank you so much.

Comment 16

[GitHub Autolander Bot]

Attachment: [gaia] ferjm:bug1142845 > mozilla-b2g:master

Comment 17

[Fernando Jiménez Moreno [:ferjm]]

(In reply to Shiwei Zhang from comment #15)
> Hi Fernando,
>  I applyed this patch in gecko on v2.1s
> https://hg.mozilla.org/mozilla-central/rev/aa2ab2b3f01b
> 
> as a result, things are going well, 5～6s are saved in exporting 1000
> contacts to SD card.
> 
> I'll examine how it works if I have time.
> 
> Thank you so much.

Great news! Thanks for testing.

Could you also test the attached PR on a dolphin with a 2.1 Gaia, please?

Thanks!

Comment 18

[Shiwei Zhang]

(In reply to Fernando Jiménez Moreno [:ferjm] from comment #17)
> (In reply to Shiwei Zhang from comment #15)
> > Hi Fernando,
> >  I applyed this patch in gecko on v2.1s
> > https://hg.mozilla.org/mozilla-central/rev/aa2ab2b3f01b
> > 
> > as a result, things are going well, 5～6s are saved in exporting 1000
> > contacts to SD card.
> > 
> > I'll examine how it works if I have time.
> > 
> > Thank you so much.
> 
> Great news! Thanks for testing.
> 
> Could you also test the attached PR on a dolphin with a 2.1 Gaia, please?
> 
> Thanks!

Dear Fernando,
 I made other tests to make sure this improvement, unfortunately, the results are not the same.
So I am sorry to say the first I should do is to check the procedures more clear. If the patch surely took effect，I‘ll do more test on a dolphin with a 2.1 Gaia later.

Comment 19

[Shiwei Zhang]

Dear Fernando,
  I am sorry to say that the test result in comment15 is not representative，and I found that the exporting performance in the first time after boot-strap was different from the second time when I reopened the Contacts app. There are 5～6s saved in the second exporting (that is why I made such a careless mistake.)
  The same thing happened on Flame. I think the reason is that selectFromList() did not cost too much time in the second exporting, but Dophine still has the bad performance issue.

Comment 20

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

Comment on attachment 8590808 [details] [review]
[gaia] ferjm:bug1142845 > mozilla-b2g:master

Redirecting to sergi, since he created that code.

Comment 21

[Fernando Jiménez Moreno [:ferjm]]

(In reply to Shiwei Zhang from comment #19)
> Dear Fernando,
>   I am sorry to say that the test result in comment15 is not
> representative，and I found that the exporting performance in the first time
> after boot-strap was different from the second time when I reopened the
> Contacts app. There are 5～6s saved in the second exporting (that is why I
> made such a careless mistake.)
>   The same thing happened on Flame. I think the reason is that
> selectFromList() did not cost too much time in the second exporting, but
> Dophine still has the bad performance issue.

That could be because we cache DB queries.

Did you try the attached Gaia patch as well?

Thanks

Comment 22

[Shiwei Zhang]

(In reply to Fernando Jiménez Moreno [:ferjm] from comment #21)
> That could be because we cache DB queries.
> 
> Did you try the attached Gaia patch as well?
> 
> Thanks

Hi Fernando,
 I have tried the Gaia patch, Unfortunately, I couldn't find a noticeable improvement in contacts exporting, the average time of exporting 1000 contacts to sdcard is 9.25s.

 I have checked the time cost in interface before (see comment1), it didn't cost too much time in accessing storage (I think that's why we can't see a noticeable improvement by applying this patch). Maybe we should pay more attentions to ContactToVcard() and ContactsExporter.init(), because these two functions are the major contributors to time consuming.

Thanks.

Comment 23

[Vance Chen [:vchen][skype:vance.lucida][[e-mail]]]

Hi Fernando -

Per Comment#22, is it possible you can spend some time to investigate if there is anything we can do in ContactToVcard() and ContactsExporter.init() to further enhance the performance?

Thanks

Comment 24

[Fernando Jiménez Moreno [:ferjm]]

I am pretty busy with other work right now, so I cannot spend more time on this for now. Maybe Thinker's team can help here.

Comment 25

[Ryan VanderMeulen [:RyanVM]]

v2.1s is EOL now. Calling this incomplete.