Created attachment 8347550 [details] logcat.txt 1. launch contacts 2. go to settings 3. turn on Sync friends 4. log into facebook Expected: facebook import finishes Actual: partial import of contacts Note: adb shell dmesg : <4>[ 3402.639558] select 1287 (Communications), adj 2, size 20343, to kill <4>[ 3402.639581] send sigkill to 1287 (Communications), adj 2, size 20343
Created attachment 8347553 [details] about-memory-3.zip I'm not sure if it's doing clean up after the kill to the app. I'm seeing it out of pmem still in the logcat. I ended up taking an about-memory after the sigkill.
Gaia 390b313a254a947d12e3cdbcde19d7d1619ff63c Gecko http://hg.mozilla.org/mozilla-central/rev/8b5875dc7e31 BuildID 20131213040203 Version 29.0a1 ro.build.version.incremental=eng.archermind.20131114.105818 ro.build.date=Thu Nov 14 10:58:33 CST 2013
The PM team traiged this and considers it a blocker
Target Milestone: --- → 1.3 C3/1.4 S3(31jan)
I've tried importing my contacts (240) and took an about:memory snapshot when importing reached ~200 contacts. Looking at it shows something very weird (I've snipped out the non-relevant parts): 65.82 MB (100.0%) -- explicit ├──23.46 MB (35.64%) -- window-objects/top(app://communications.gaiamobile.org/contacts/index.html, id=1) │ ├──21.48 MB (32.63%) -- active │ │ ├──15.81 MB (24.01%) -- window(app://communications.gaiamobile.org/contacts/index.html) │ │ │ ├──14.13 MB (21.47%) -- js-compartment(app://communications.gaiamobile.org/contacts/index.html) │ │ │ │ ├──13.50 MB (20.51%) -- objects │ │ │ │ │ ├──12.74 MB (19.36%) -- malloc-heap │ │ │ │ │ │ ├──12.20 MB (18.53%) ── elements/non-asm.js │ │ ├───4.63 MB (07.04%) -- window(app://communications.gaiamobile.org/contacts/import.html?service=facebook) │ │ │ ├──3.05 MB (04.63%) -- js-compartment(app://communications.gaiamobile.org/contacts/import.html?service=facebook) │ │ │ │ ├──2.43 MB (03.70%) -- objects │ │ │ │ │ ├──1.84 MB (02.79%) -- malloc-heap │ │ │ │ │ │ ├──1.75 MB (02.66%) ── elements/non-asm.js ├──17.65 MB (26.81%) -- js-non-window │ ├──11.57 MB (17.58%) -- zones │ │ ├──10.35 MB (15.72%) -- zone(0x40396800) │ │ │ ├───9.19 MB (13.96%) -- compartment([System Principal]) │ │ │ │ ├──8.09 MB (12.30%) -- objects │ │ │ │ │ ├──6.71 MB (10.20%) -- malloc-heap │ │ │ │ │ │ ├──6.27 MB (09.53%) ── elements/non-asm.js The elements/non-asm.js portions keep growing during the import and they account for dynamically added object properties. So if I'm not mistaken it seems that we're dynamically *LOTS* adding properties to one or more objects. So many that we eventually run into an OOM condition. I'll have to investigate further tomorrow but I get the feeling that all the import-from-Facebook bugs might be related to this.
Whiteboard: [c=memory p= s= u=1.3] → [c=memory p= s= u=1.3][MemShrink]
thanks Andrea. I can confirm that the problem is within the code that indexes FB contacts by phone number (tel_indexer.js). If no tel numbers are present the import process can work seamlessly with more than 1000 contacts. so I'm going to re implement the index in order not to create so many objects. Thus, I'm stealing the bug
(In reply to Jose M. Cantera from comment #6) > I can confirm that the problem is within the code that > indexes FB contacts by phone number (tel_indexer.js). If no tel numbers are > present the import process can work seamlessly with more than 1000 contacts. > so I'm going to re implement the index in order not to create so many > objects. Thus, I'm stealing the bug Thanks, I was still looking around the code but hadn't identified where it was coming from yet. I think the issue here is that you're building the tree nodes by dynamically setting their properties, effectively using them as hash-tables. This is unfortunately very expensive both from a memory and CPU POV. You're better off if you build your tree structures in a C/C++-ish style with fixed-layout objects & native arrays.
In order to fix properly this bug not only we need to reimplement the telephone index but also fix bug 950881, otherwise the performance when importing friends is comic and the risk for OOM kill higher. This is due to the fact that we are saving the index (a big tree) everytime a new friend is added.
Whiteboard: [c=memory p= s= u=1.3][MemShrink] → [c=memory p= s= u=1.3][MemShrink:P2]
apart from the recommendation from Gabriele I believe performance and memory will benefit from fixing also bug 950881.
Whiteboard: [c=memory p= s= u=1.3][MemShrink:P2] → [c=memory p= s= u=1.3][MemShrink:P2][mwcdemo2014]
Created attachment 8365102 [details] 15686.html the telephone index has been redesigned in order to have less nodes and using arrays as much as possible instead of object properties
Attachment #8365102 - Flags: review?(crdlc)
Comment on attachment 8365102 [details] 15686.html Hi, the code looks good to me although there are some comments on github. Good job
Attachment #8365102 - Flags: review?(crdlc) → review+
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
I was not able to uplift this bug to v1.3. If this bug has dependencies which are not marked in this bug, please comment on this bug. If this bug depends on patches that aren't approved for v1.3, we need to re-evaluate the approval. Otherwise, if this is just a merge conflict, you might be able to resolve it with: git checkout v1.3 git cherry-pick -x -m1 4516f0287c3f817c14aa32b8af4d577121e62240 <RESOLVE MERGE CONFLICTS> git commit
We cannot uplift this one until bug 950881 is uplifted, which now depends on another uplift. thanks
uplifted to v1.3: https://github.com/mozilla-b2g/gaia/commit/1561b1c8bb5e8bce90dc93a9e3e1ccb6e7b927e2
status-b2g-v1.3: affected → fixed
John Hammink, can you please take care of this one?
Checked this out, and it works after two full imports of ~ 1500 contacts Gaia 8039a5cb7519adfa81677df577f494c6a4de6140 Gecko https://hg.mozilla.org/releases/mozilla-b2g28_v1_3/rev/e5f25becc0e7 BuildID 20140221004002 Version 28.0 ro.build.version.incremental=324 ro.build.date=Thu Dec 19 14:04:55 CST 2013
Status: RESOLVED → VERIFIED
status-b2g-v1.3T: --- → fixed
status-b2g-v1.4: --- → fixed
You need to log in before you can comment on or make changes to this bug.