[dolphin][perf] improve the performance of exporting Contacts to USIM card (china unicom)

RESOLVED FIXED in Firefox 33, Firefox OS v1.4

Status

Firefox OS
RIL
P1
blocker
RESOLVED FIXED
4 years ago
3 years ago

People

(Reporter: Peipei Cheng (needinfo if you need my action), Assigned: edgar)

Tracking

(Blocks: 2 bugs, {perf})

unspecified
2.0 S6 (18july)
ARM
Gonk (Firefox OS)
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(blocking-b2g:1.4+, firefox31 wontfix, firefox32 wontfix, firefox33 fixed, b2g-v1.4 fixed, b2g-v2.0 wontfix, b2g-v2.1 fixed)

Details

(Whiteboard: [c=progress p= s= u=1.4,dolphin] [sprd316455][partner-blocker])

Attachments

(2 attachments, 4 obsolete attachments)

Here is the data for exporting 500 contacts to China Unicom USIM card. China Mobile SIM card does not have this problem.
Dolphin: 346.87s (Flame has similar data)
7715 android: 198s
android 6821: 320s

Spreadtrum did following experiment.

Add console log in /gaia/apps/communications/contacts/js/export/sim.js
//console.log('---start icc.update ---');
var request = icc.updateContact('adn', theContact);
request.onsuccess = function onsuccess() { 
//console.log('---end icc.update ---');
}

It turns out that icc.updateContact() took too much time (about 2s for each contact). Please see the logs bellow:

05-14 17:33:25.054   957   957 E GeckoConsole: Content JS LOG at app://communications.gaiamobile.org/contacts/js/export/sim.js:135 in _doExport: ---start icc.update ---
05-14 17:33:27.457   957   957 E GeckoConsole: Content JS LOG at app://communications.gaiamobile.org/contacts/js/export/sim.js:140 in onsuccess: ---end icc.update ---
05-14 17:33:27.627   957   957 E GeckoConsole: Content JS LOG at app://communications.gaiamobile.org/contacts/js/export/sim.js:135 in _doExport: ---start icc.update ---
05-14 17:33:30.199   957   957 E GeckoConsole: Content JS LOG at app://communications.gaiamobile.org/contacts/js/export/sim.js:140 in onsuccess: ---end icc.update ---
05-14 17:33:30.369   957   957 E GeckoConsole: Content JS LOG at app://communications.gaiamobile.org/contacts/js/export/sim.js:135 in _doExport: ---start icc.update ---
05-14 17:33:32.992   957   957 E GeckoConsole: Content JS LOG at app://communications.gaiamobile.org/contacts/js/export/sim.js:140 in onsuccess: ---end icc.update ---
05-14 17:33:33.162   957   957 E GeckoConsole: Content JS LOG at app://communications.gaiamobile.org/contacts/js/export/sim.js:135 in _doExport: ---start icc.update ---
05-14 17:33:36.005   957   957 E GeckoConsole: Content JS LOG at app://communications.gaiamobile.org/contacts/js/export/sim.js:140 in onsuccess: ---end icc.update ---


Reading USIM also has similar issue. Please see also: Bug 1005609 - [Tarako][Dolphin][Perf][ImportContact] It takes a long time to import 50 contacts from a China Unicom SIM card
Keywords: perf
Whiteboard: [sprd316455][partner-blocker]
blocking-b2g: --- → 1.4?
QA Wanted to check 1.3.
Component: General → Gaia::Contacts
Ivan

Can we please verify against a Buri device?
Flags: needinfo?(itsay)
OS: Linux → Gonk (Firefox OS)
Priority: -- → P1
Hardware: x86_64 → ARM
Whiteboard: [sprd316455][partner-blocker] → [sprd316455][partner-blocker][c=progress p= s= u=1.4]

Updated

4 years ago
Keywords: qawanted
status-b2g-v1.4: --- → affected
Flags: needinfo?(kevin)

Comment 3

4 years ago
Hi Pei-pei,

How's the performance in Buri v1.4.
Flags: needinfo?(itsay) → needinfo?(pcheng)

Updated

4 years ago
Whiteboard: [sprd316455][partner-blocker][c=progress p= s= u=1.4] → [c=progress p= s= u=dolphin] [sprd316455][partner-blocker]
Flags: needinfo?(kevin) → needinfo?(janjongboom)
(In reply to Ivan Tsay (:ITsay) from comment #3)
> Hi Pei-pei,
> 
> How's the performance in Buri v1.4.

I think Buri has the similar data as Flame.
We found icc.updateContact() and mozContacts.save() use most time. Every contact use about 6s in mozContacts.save().
Ran a small test with 200 contacts and we slow down over time depending on the number of contacts already on the SIM (at least that's the case on the vala sim I have here).

First contact export to SIM are 300 ms. (Peak 1.4), but goes to 2 seconds after 100. Then 4 seconds after 200.
Exporting some more later on won't be faster, also takes 4 seconds.

We should monitor the time spent in RIL (time spent in RILContentHelper.js between updateContact and handleUpdateIccContact) and see if that increases as well, or that it's something in Gecko. I'm waiting for my B2G build to finish before I can do that tho.
Assignee: nobody → janjongboom
Flags: needinfo?(janjongboom)
Jan, I don't think it's RIL issue, because dolphin android is faster than dolphin firefox.
Dolphin: 346.87s (Flame has similar data)
Dolphin: android: 198s
Blocks: 1005609
Flags: needinfo?(james.zhang)

Comment 8

4 years ago
All, Please do not name partners and products explicitly.
Peipei - I'm having trouble analyzing the data in comment 0. Questions:

1. Can you give comparable perf analysis on Buri 1.3?
2. Which Android chipset should we be comparing against (7715 or 6821)?
This is specific to China Unicom USIM card . Should be the same root cause with Bug 1005609. Please see my comment bellow. Also different type of USIM has different performance, so my test result is worse than spreadtrum data.

(In reply to Jason Smith [:jsmith] from comment #9)
> Peipei - I'm having trouble analyzing the data in comment 0. Questions:
> 
> 1. Can you give comparable perf analysis on Buri 1.3?
I tested on buri V1.3 1.4: exporting 191 contacts to China Unicom USIM card took over 16 minutes.

I saw similar behavior as comment 5. At the beginning, the avg speed is fast, and then it become slower and slower for exporting more (about 5s or 6s for each contact).

> 2. Which Android chipset should we be comparing against (7715 or 6821)?
The performance test use 7715 chipset as baseline, which is 198s for 500 contacts.
Flags: needinfo?(pcheng)
Flags: needinfo?(james.zhang)

Comment 11

4 years ago
Wayne & Preeti,

Does this meet the criteria for Dolphin blockers? Dolphin performance is better than Buri.
Status: NEW → ASSIGNED
Flags: needinfo?(wchang)
Flags: needinfo?(praghunath)
I'd rather get this fixed. But lets fix it on 1.4 Dolphin branch
Flags: needinfo?(praghunath)
Whiteboard: [c=progress p= s= u=dolphin] [sprd316455][partner-blocker] → [c=progress p= s= u=dolphin] [sprd316455][partner-blocker][dolphin land]
It seems there is nothing here the QA-Wanted team can do as this issue appears to be specific to a SIM we do not have (China Unicom USIM card) so I'm striking QA-Wanted. If this is in error please feel free to re-add it.
Keywords: qawanted

Updated

4 years ago
Target Milestone: --- → 2.0 S5 (4july)
(In reply to Jan Jongboom [:janjongboom] (Telenor) from comment #5)
> Ran a small test with 200 contacts and we slow down over time depending on
> the number of contacts already on the SIM (at least that's the case on the
> vala sim I have here).
> 
> First contact export to SIM are 300 ms. (Peak 1.4), but goes to 2 seconds
> after 100. Then 4 seconds after 200.
> Exporting some more later on won't be faster, also takes 4 seconds.
> 
> We should monitor the time spent in RIL (time spent in RILContentHelper.js
> between updateContact and handleUpdateIccContact) and see if that increases
> as well, or that it's something in Gecko. I'm waiting for my B2G build to
> finish before I can do that tho.


Jan, have you been able to find out more here?
If this is China Unicom USIM specific as described in comment 10 and comment 13, I don't think we should block on this. However if this is generic then we would definitely prefer to have it fixed.
Flags: needinfo?(wchang) → needinfo?(janjongboom)
My results come from a Vala SIM, but all time was spent in qcom RIL, not in Gecko as far as I can see through my printf debugging.
Flags: needinfo?(janjongboom) → needinfo?(wchang)
Shawn,

Can you check this together with Jan? It seems to impact projects for different chipsets.

Thanks
Flags: needinfo?(wchang) → needinfo?(sku)
Blocking for 1.4
blocking-b2g: 1.4? → 1.4+

Updated

4 years ago
Whiteboard: [c=progress p= s= u=dolphin] [sprd316455][partner-blocker][dolphin land] → [c=progress p= s= u=1.4,dolphin] [sprd316455][partner-blocker][dolphin land]
What's the status of this blocker, Jan do you need any help here?
Flags: needinfo?(janjongboom)
Ah sorry, didn't unassign myself. This was about as far as I could get, need someone with more knowledge of this code to look at it.
Assignee: janjongboom → nobody
Flags: needinfo?(janjongboom)

Comment 20

4 years ago
Hi Peipei:
 Do we have both device/radio log to check the bottleneck since this issue only happens on "China Unicom USIM card"?

Thanks!!
Shawn
Flags: needinfo?(sku) → needinfo?(pcheng)
I tested again in Flame v1.4. It took over 30 minutes to export 193 contacts to China Unicom USIM card so the adb log is very big.

Please find the adb logcat for radio and main here: https://www.dropbox.com/s/z8gk08k27ukwodo/1022490.log

Tests starts: 06-30 04:02:45.905
Flags: needinfo?(pcheng)

Comment 22

4 years ago
It looks rild/modem take 2.x to 15.x sec for single entry update.
(getting slow after more and more manipulation on sim phonebook)

// Log snippet
06-30 04:03:08.515 I/Gecko   (  319): RIL Worker: [0] Update ICC Contact {"alphaId":"Aaron M. Crome","number":"02657360277","email":"aaronmcromer@spambob.com","contactId":"null"}
06-30 04:03:11.725 I/Gecko   ( 1406): -*- RILContentHelper: Received message 'RIL:UpdateIccContact': {"clientId":0,"data":{"requestId":"id{d10cc290-ae77-4874-ad83-11125bb63a4e}","contactType":"adn","contact":{"alphaId":"Aaron M. Crome","number":"02657360277","email":"aaronmcromer@spambob.com","contactId":"898601138110237789833","pbrIndex":0,"recordId":3},"pin2":null,"rilMessageClientId":0,"rilMessageToken":27,"rilMessageType":"updateICCContact"}}

...

06-30 04:03:11.835 I/Gecko   (  319): RIL Worker: [0] Update ICC Contact {"alphaId":"Ada D. Banks","number":"61601894","email":"AdaDBanks@mailinator.com","contactId":"null"}
06-30 04:03:14.585 I/Gecko   ( 1406): -*- RILContentHelper: Received message 'RIL:UpdateIccContact': {"clientId":0,"data":{"requestId":"id{3f18750d-75ce-48d4-af8b-ab069c4c5c25}","contactType":"adn","contact":{"alphaId":"Ada D. Banks","number":"61601894","email":"AdaDBanks@mailinator.com","contactId":"898601138110237789834","pbrIndex":0,"recordId":4},"pin2":null,"rilMessageClientId":0,"rilMessageToken":28,"rilMessageType":"updateICCContact"}}

...

06-30 04:03:14.695 I/Gecko   (  319): RIL Worker: [0] Update ICC Contact {"alphaId":"Adam L. Card","number":"0267397915","email":"AdamLCard@trashymail.com","contactId":"null"}
06-30 04:03:17.445 I/Gecko   ( 1406): -*- RILContentHelper: Received message 'RIL:UpdateIccContact': {"clientId":0,"data":{"requestId":"id{f7998901-e9c9-4acf-80d7-adedd25c83ea}","contactType":"adn","contact":{"alphaId":"Adam L. Card","number":"0267397915","email":"AdamLCard@trashymail.com","contactId":"898601138110237789835","pbrIndex":0,"recordId":5},"pin2":null,"rilMessageClientId":0,"rilMessageToken":29,"rilMessageType":"updateICCContact"}}

...

06-30 04:21:40.425 I/Gecko   (  319): RIL Worker: [0] Update ICC Contact {"alphaId":"丽 刘","number":"+8618675832593","contactId":"null"}
06-30 04:21:50.115 I/Gecko   ( 1406): -*- RILContentHelper: Received message 'RIL:UpdateIccContact': {"clientId":0,"data":{"requestId":"id{1d661d1f-c7f9-4f0a-b98e-ff09c953765a}","contactType":"adn","contact":{"alphaId":"丽 刘","number":"+8618675832593","contactId":"89860113811023778983112","pbrIndex":0,"recordId":112},"pin2":null,"rilMessageClientId":0,"rilMessageToken":138,"rilMessageType":"updateICCContact"}}

...

6-30 04:21:50.205 I/Gecko   (  319): RIL Worker: [0] Update ICC Contact {"alphaId":"丽 吴","number":"+8613913008184","contactId":"null"}
06-30 04:21:59.765 I/Gecko   ( 1406): -*- RILContentHelper: Received message 'RIL:UpdateIccContact': {"clientId":0,"data":{"requestId":"id{4a482bd4-cebe-4d8e-905a-b13fd63ef0d7}","contactType":"adn","contact":{"alphaId":"丽 吴","number":"+8613913008184","contactId":"89860113811023778983113","pbrIndex":0,"recordId":113},"pin2":null,"rilMessageClientId":0,"rilMessageToken":139,"rilMessageType":"updateICCContact"}}

...

06-30 04:21:59.865 I/Gecko   (  319): RIL Worker: [0] Update ICC Contact {"alphaId":"丽莎 朱","number":"+8613601588296","contactId":"null"}
06-30 04:22:09.665 I/Gecko   ( 1406): -*- RILContentHelper: Received message 'RIL:UpdateIccContact': {"clientId":0,"data":{"requestId":"id{da436ea3-0514-4927-8eb0-6cf54ccda406}","contactType":"adn","contact":{"alphaId":"丽莎 朱","number":"+8613601588296","contactId":"89860113811023778983114","pbrIndex":0,"recordId":114},"pin2":null,"rilMessageClientId":0,"rilMessageToken":140,"rilMessageType":"updateICCContact"}}

...

06-30 04:22:09.775 I/Gecko   (  319): RIL Worker: [0] Update ICC Contact {"alphaId":"举 鹏 陈","number":"+8613811403082","contactId":"null"}
06-30 04:22:19.465 I/Gecko   ( 1406): -*- RILContentHelper: Received message 'RIL:UpdateIccContact': {"clientId":0,"data":{"requestId":"id{d19fab02-bd57-45f2-8e1a-acd093bc99fc}","contactType":"adn","contact":{"alphaId":"举 鹏 陈","number":"+8613811403082","contactId":"89860113811023778983115","pbrIndex":0,"recordId":115},"pin2":null,"rilMessageClientId":0,"rilMessageToken":141,"rilMessageType":"updateICCContact"}}

...


06-30 04:37:51.265 I/Gecko   (  319): RIL Worker: [0] Update ICC Contact {"alphaId":"隽 曹","number":"15861161819","contactId":"null"}
06-30 04:38:06.445 I/Gecko   ( 1406): -*- RILContentHelper: Received message 'RIL:UpdateIccContact': {"clientId":0,"data":{"requestId":"id{79290b95-e970-4712-8962-107758cbfd37}","contactType":"adn","contact":{"alphaId":"隽 曹","number":"15861161819","contactId":"89860113811023778983191","pbrIndex":0,"recordId":191},"pin2":null,"rilMessageClientId":0,"rilMessageToken":217,"rilMessageType":"updateICCContact"}}

...

06-30 04:38:06.555 I/Gecko   (  319): RIL Worker: [0] Update ICC Contact {"alphaId":"青云 施","number":"13588746882","contactId":"null"}
06-30 04:38:21.835 I/Gecko   ( 1406): -*- RILContentHelper: Received message 'RIL:UpdateIccContact': {"clientId":0,"data":{"requestId":"id{be162f65-4842-48a5-bef0-0a8ff91be3cb}","contactType":"adn","contact":{"alphaId":"青云 施","number":"13588746882","contactId":"89860113811023778983192","pbrIndex":0,"recordId":192},"pin2":null,"rilMessageClientId":0,"rilMessageToken":218,"rilMessageType":"updateICCContact"}}

...

06-30 04:38:21.945 I/Gecko   (  319): RIL Worker: [0] Update ICC Contact {"alphaId":"静 程","number":"+8618668413976","contactId":"null"}
06-30 04:38:37.495 I/Gecko   ( 1406): -*- RILContentHelper: Received message 'RIL:UpdateIccContact': {"clientId":0,"data":{"requestId":"id{3f16bca3-596f-4c06-a386-13c440e4678d}","contactType":"adn","contact":{"alphaId":"静 程","number":"+8618668413976","contactId":"89860113811023778983193","pbrIndex":0,"recordId":193},"pin2":null,"rilMessageClientId":0,"rilMessageToken":219,"rilMessageType":"updateICCContact"}}

Comment 23

4 years ago
Hi Sam:
 Could you please help provide 7715 Android device/radio log for us to check?

Repo. request:
Export 300 contacts to CU USIM.
(device/radio log are both needed.)

// Command
adb logcat -b radio -b main -v threadtime > /tmp/android.log

Thanks!!
Shawn
Flags: needinfo?(sam.hua)

Comment 24

4 years ago
Shawn, it looks like you're leading up this blocker, so I am assigning to you. If this is incorrect, please reassign.

FxOS Performance Triage
Assignee: nobody → sku

Comment 25

4 years ago
Hi Shawn,
sorry for late,i will provide it today.

Comment 26

4 years ago
Created attachment 8450727 [details]
Android_7715GA_export_contacts.rar

android log
Flags: needinfo?(sam.hua)

Updated

4 years ago
Flags: needinfo?(sku)

Comment 27

4 years ago
it looks like partner has it own flow for exporting USIM pbk.

07-04 10:28:30.827   948  1337 V IccPhoneBookInterfaceManager: updateAdnRecordsInEfBySearchEx** 
07-04 10:28:30.837   948  1337 D IccPhoneBookInterfaceManager: isPbrFileExisting = true
07-04 10:28:30.837   948  1337 I AdnRecordCache: updateUSIMAdnBySearch efid 4f30
07-04 10:28:30.837   948  1337 I UsimPhoneBookManager: findExtensionEFInfo fileIds {201=20321, 200=20300, 203=20303, 202=20337, 197=20289, 196=20314, 199=20299, 198=20305, 193=20273, 192=20281, 194=20298}
07-04 10:28:30.837   948  1337 E AdnRecordCache: efid : 20281extensionEF :20298 iapEF:20273
07-04 10:28:30.837   948  1337 I AdnRecordCache: updateUSIMAdnBySearch (1)
07-04 10:28:30.837   948  1337 E AdnRecordCache: efid is 20281
07-04 10:28:30.837   948  1337 I AdnRecordCache: updateUSIMAdnBySearch (2)
07-04 10:28:30.837   948  1337 D AdnRecordCache: we got the index 3
07-04 10:28:30.837   948  1337 I AdnRecordCache: updateUSIMAdnBySearch (3)
07-04 10:28:30.837   948  1337 I AdnRecordCache: mInsertId3
07-04 10:28:30.837   948  1337 I UsimPhoneBookManager: getSubjectEfids  length 1
07-04 10:28:30.837   948  1337 I AdnRecordCache: Begin : updateSubjectOfAdn num =0 adnNum 3 index 3
07-04 10:28:30.837   948  1337 D AdnRecordCache: updateEmailResult = 0
07-04 10:28:30.837   948  1337 I UsimPhoneBookManager: getSubjectEfids  length 1
07-04 10:28:30.837   948  1337 I AdnRecordCache: Begin : updateSubjectOfAdn num =0 adnNum 3 index 3

Comment 28

4 years ago
Hi Ken:
 Could you please have your team member to check if any improvement for this topic?

Thanks!!
Shawn
Flags: needinfo?(kchang)

Comment 29

4 years ago
Edgar, can we do anything for this bug?
Flags: needinfo?(sku)
Flags: needinfo?(kchang)
Flags: needinfo?(echen)
(Assignee)

Comment 30

4 years ago
(In reply to Ken Chang[:ken] from comment #29)
> Edgar, can we do anything for this bug?

Currently we search a free slot for the new contact from the first entry every time.
I guess catching the last index of free slot can improve the performance. I can have a try first, but I don't know how much that would help.
Assignee: sku → echen
Flags: needinfo?(echen)

Comment 31

4 years ago
Maybe we can cache the ADN record first, and do the similar thing as AOSP did to locate the empty record via cache. We can reduce the time on every findFreeICCContact with type ICC_EF_ADN Thereafter.
Target Milestone: 2.0 S5 (4july) → 2.0 S6 (18july)
(Assignee)

Comment 32

4 years ago
Created attachment 8451579 [details] [diff] [review]
WIP, patch, v1

(In reply to Edgar Chen [:edgar][:echen] from comment #30)
> Currently we search a free slot for the new contact from the first entry
> every time.
> I guess catching the last index of free slot can improve the performance. I
> can have a try first, but I don't know how much that would help.

Hi Peipei/Sam,

I would like to know how much this idea would help. Could you help to test Dolphin with this patch and also test again without this patch? Please use the same sim card and clear all sim contact first for both test.

Thank you.
Flags: needinfo?(sam.hua)
(Assignee)

Updated

4 years ago
Flags: needinfo?(pcheng)
Yang, please land this WIP patch and verify, thanks.
Flags: needinfo?(yang.zhao)
Flags: needinfo?(pcheng)

Comment 34

4 years ago
(In reply to James Zhang (Spreadtrum) from comment #33)
> Yang, please land this WIP patch and verify, thanks.

Already landed it under sprd_patch,commit id:dffad04cdbef5775891fcd1cef7584b5fd1ee1ee
Flags: needinfo?(yang.zhao)

Updated

4 years ago
Severity: normal → blocker
(Assignee)

Comment 35

4 years ago
Created attachment 8453667 [details] [diff] [review]
Patch, v2

In this patch, I modify the logic of searching free recorder a bit. The main idea is simply to assume the recorder next to the previous result is also free.
So we could just start searching from it next time, if we are lucky enough, we can find the free recorder immediately. 
For the "no free recorder" case, we should do a cyclic search to make sure we have searched all recorder as well as phonebook sets before we file a "CONTACT_ERR_NO_FREE_RECORD_FOUND" error.

Hi Yoshi, may I have your review? Thank you.
Attachment #8451579 - Attachment is obsolete: true
Attachment #8453667 - Flags: review?(allstars.chh)
(Assignee)

Comment 36

4 years ago
Share my test result in emulator (run marionette-test for exporting contacts to SIM/USIM)

1). Exporting 200 contacts to SIM:
- Without improvement: 110s
- With improvement: 14s

2). Exporting 200 contacts to USIM:
- Without improvement: 112s
- With improvement: 18s

3). Exporting 350 contacts to USIM:
- Without improvement: 301s
- With improvement: 30s

*Note: The above measurements are based on emulator+marionette-test, so the result may be different with real device.

Thank you.
(Assignee)

Updated

4 years ago
Component: Gaia::Contacts → RIL
Summary: [dolphin][flame][perf] improve the performance of exporting Contacts to USIM card (china unicom) → [dolphin][perf] improve the performance of exporting Contacts to USIM card (china unicom)
Comment on attachment 8453667 [details] [diff] [review]
Patch, v2

Review of attachment 8453667 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/system/gonk/ril_worker.js
@@ +12790,1 @@
>        } else {

when will nextRecord == recordNumber?

@@ +12791,5 @@
>          // No free record found.
>          if (DEBUG) {
>            this.context.debug(CONTACT_ERR_NO_FREE_RECORD_FOUND);
>          }
>          onerror(CONTACT_ERR_NO_FREE_RECORD_FOUND);

here should we also clear this._freeRecordIds[fileId]?
Attachment #8453667 - Flags: review?(allstars.chh)
(Assignee)

Comment 38

4 years ago
(In reply to Yoshi Huang[:allstars.chh] from comment #37)
> Comment on attachment 8453667 [details] [diff] [review]
> Patch, v2
> 
> Review of attachment 8453667 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/system/gonk/ril_worker.js
> @@ +12790,1 @@
> >        } else {
> 
> when will nextRecord == recordNumber?

We do a cyclic search here.

For example, 
if this._freeRecordIds[fileId] is 10, we will do searching in following sequence until we find a free one.

10, 11, 12 ...., 252, 253, 254, 1, 2, 3, ..., 7, 8, 9

If all records are not free, we stop at 9 and file "CONTACT_ERR_NO_FREE_RECORD_FOUND" error.

> 
> @@ +12791,5 @@
> >          // No free record found.
> >          if (DEBUG) {
> >            this.context.debug(CONTACT_ERR_NO_FREE_RECORD_FOUND);
> >          }
> >          onerror(CONTACT_ERR_NO_FREE_RECORD_FOUND);
> 
> here should we also clear this._freeRecordIds[fileId]?

Ah, yes, clear this._freeRecordIds[fileId] then do searching from the first record next time seems better. Will do this in next version, thank you.
Comment on attachment 8453667 [details] [diff] [review]
Patch, v2

Review of attachment 8453667 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/system/gonk/ril_worker.js
@@ +12768,5 @@
>            break;
>          }
>        }
>  
> +      let nextRecord = (options.p1 % options.totalRecords) + 1;

(options.p1 + 1) % options.totalRecords; is more easier to understand.

@@ +14374,5 @@
>  
> +  /**
> +   * Cache the pbr index of the possible free recorder.
> +   */
> +  _freeRecordPbrIndex: 0,

_freePbrIndex is enough
Attachment #8453667 - Flags: review+
(Assignee)

Comment 40

4 years ago
Created attachment 8454338 [details] [diff] [review]
Patch, v3, r=allstars.chh

- Address review comment #38 and comment #39.
- carry r=allstars.chh after r+
Attachment #8453667 - Attachment is obsolete: true
Attachment #8454338 - Flags: review+
(Assignee)

Comment 41

4 years ago
Created attachment 8455116 [details] [diff] [review]
Changes for xpcshell tests
(Assignee)

Comment 42

4 years ago
Comment on attachment 8455116 [details] [diff] [review]
Changes for xpcshell tests

In findFreeRecordId(), now we call context.RIL.iccIO() directly instead of ICCIOHelper.loadNextRecord(). We need to do the same change in test_ril_worker_icc_ICCRecordHelper.
Hi Yoshi, could you help to review this, thank you.
Attachment #8455116 - Flags: review?(allstars.chh)
Attachment #8455116 - Flags: review?(allstars.chh) → review+
(Assignee)

Comment 43

4 years ago
Created attachment 8455156 [details] [diff] [review]
Patch, v4, r=allstars.chh

Squash attachment 8454338 [details] [diff] [review] and attachment 8455116 [details] [diff] [review] for landing.
Attachment #8454338 - Attachment is obsolete: true
Attachment #8455116 - Attachment is obsolete: true
Attachment #8455156 - Flags: review+

Comment 45

4 years ago
Hi Edgar,
If u want to verify this issue in Spreadtrum, please need info me or Yang.zhao
Flags: needinfo?(sam.hua) → needinfo?(echen)
(Assignee)

Comment 46

4 years ago
(In reply to sam.hua from comment #45)
> Hi Edgar,
> If u want to verify this issue in Spreadtrum, please need info me or
> Yang.zhao

Hi Sam, I would like to know how much this patch will help, if you could provide the result, it will be great, thank you. (See comment #32)
Flags: needinfo?(echen) → needinfo?(sam.hua)
https://hg.mozilla.org/mozilla-central/rev/189d66b64dfd
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
https://hg.mozilla.org/releases/mozilla-b2g30_v1_4/rev/de7ecfb00955
status-b2g-v1.4: affected → fixed
status-b2g-v2.0: --- → affected
status-b2g-v2.1: --- → fixed
status-firefox31: --- → wontfix
status-firefox32: --- → affected
status-firefox33: --- → fixed
Whiteboard: [c=progress p= s= u=1.4,dolphin] [sprd316455][partner-blocker][dolphin land] → [c=progress p= s= u=1.4,dolphin] [sprd316455][partner-blocker]

Comment 50

4 years ago
it cost 3'26" to export 415 contacts to SIM card.
Flags: needinfo?(sam.hua) → needinfo?(echen)

Updated

4 years ago
Duplicate of this bug: 1005609
status-firefox32: affected → wontfix
ni on me to verify.
Flags: needinfo?(pcheng)
The data in comment 0 comes from spreadtrum QA. The test result may differ due to different type of USIM.

I used the same type of USIM for my tests. There is good improvement on both Flame and Dolphin:

Flame 20140714000206 (before fix)
   --> Exporting 100 Contacts took 15 min 36 s. I can see exporting one contact become slower and slower
Flame 20140724180352 (after fix)
   --> Exporting 100 Contacts took 4 min 12s (about 2s for each contact), the speed looks stable to me

Dolphin 20140713160202 (before fix)
   --> Exporting 100 Contacts took 7 min, I can see the speed become slower and slower.
Dolphin 20140714160202 (after fix)
   --> Exporting 100 Contacts took 2 min.


James/Rachelle, I think we should leave this bug closed now. if spreadtrum think the speed is still not acceptable, we need a new bug to follow it up.
Flags: needinfo?(pcheng)
Hi Peipei, thanks a lot for your valuable update. Agree with you.


Hi James, please kindly provide sprd testing results.Thanks!
If it does improves a lot with the patch, this bug should not be considered as a blocker anymore.Thanks !
(Assignee)

Comment 55

4 years ago
Thanks for the update, Peipei.
Flags: needinfo?(echen)
Note that per the recent rule change announced in dev-gaia, 1.4 blockers don't have auto-approval to land on v2.0. Please nominate this patch for b2g32 uplift if you feel that it needs to land there as well. Sorry for the inconvenience :(
Flags: needinfo?(echen)
(Assignee)

Comment 57

4 years ago
We don't need to land on v2.0 right now. I will ask approval if we need it later. Thank you, Ryan. :)
Flags: needinfo?(echen)
status-b2g-v2.0: affected → wontfix
(Assignee)

Updated

4 years ago
Duplicate of this bug: 1048122
(Assignee)

Updated

3 years ago
Blocks: 1157082
You need to log in before you can comment on or make changes to this bug.