Closed Bug 835791 Opened 11 years ago Closed 11 years ago
[Contacts] Improve opening time performance
355 bytes, text/html
We need to reduce as much as possible all the content that is initially loaded and is not needed for the initial screen
Assignee: nobody → alberto.pastor
blocking-b2g: --- → tef?
Without a more concrete plan or issue here, we can't block so I'm going to mark is as tracking.
Pointer to Github pull-request
Maybe moving this back to tef? was a mistake? :)
blocking-b2g: tef? → -
I was informed offline that there is a separate triage session for performance-sensitive issues and that this should probably be a blocker.
blocking-b2g: - → tef+
Marking status-b2g18 and status-b2g18-v1.0.0 as affected, please update the status to fixed once this is verified landed on v1-train/mozilla-b2g18 and v1.0.0/mozilla-b2g18_v_1_0_0
Any update on this ?
I started yesterday working on this. I already have a patch for async loading Facebook and settings stuff. It decreases already loading time :) Even though, it can be decreased even more avoiding .getAll from facebook indexdb and other minor improvements. I'll work on that tomorrow.
(In reply to Alberto Pastor from comment #7) > I started yesterday working on this. I already have a patch for async > loading Facebook and settings stuff. It decreases already loading time :) > > Even though, it can be decreased even more avoiding .getAll from facebook > indexdb and other minor improvements. I'll work on that tomorrow. great Alberto. Let me know if you need some help with that and in any case ask me for a review for that part thanks!
WIth the patch applied === Contacts average load time: 1280ms
Comments made on the code, patch looking good, just waiting for address some changes regarding syncing when fb is completely loaded.
Attachment #712386 - Flags: review?(crdlc) → review+
the patch needs to be rebased to catch up with the latest Contact settings changes made for SIM Import alignment with new progress BBs
Comment on attachment 712386 [details] Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/8038 Nice work, I guess since this already needs to be rebased we can wait till bug 836170, hopefully will land tomorrow. Cheers!
Attachment #712386 - Flags: review?(francisco.jordano) → review+
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Batch edit: Bugs still affected on b2g18 after 2/13 merge to v1.0.1 branch are affected on v1.0.1 branch.
This commit does not apply cleanly to v1-train or v1.0.1. If this patch depends on another bug, please comment here and I will retry when that bug is approved to land on all branches that this bug needs to land on. If the merge conflict needs to be resolved by hand, the following commands could be a useful starting point: cd gaia git checkout v1-train git cherry-pick -x -m1 e6f1589e29a0b1163a42d14954f1df5f03291450 <resolve merge conflict> git checkout v1.0.1 git cherry-pick -x $(git log --pretty=%H -n1 v1-train) The following files needed merging: # both modified: apps/communications/contacts/index.html # both modified: apps/communications/contacts/js/contacts.js # both modified: apps/communications/contacts/js/contacts_list.js
Alberto, can you take a look at this ASAP please? If this fix is not uplifted it will not make the release.
Hi, It seems it depends on Bug 823025, and apart from that, there were some conflicts recording Performance testing scripts (that I guess don't need to land here). cd gaia git checkout v1-train git pull https://github.com/albertopq/gaia.git merge-contacts-performance-opening That would merge both bugs without conflicts. I've tested it on a device, and everything works as expected. Regards,
BTW, Bug 823025 is not strictly a dependency, just a collision regarding changed lines. If you prefer me creating a patch without that bug, I can do it as well. Regards
(In reply to Alberto Pastor from comment #19) > BTW, Bug 823025 is not strictly a dependency, just a collision regarding > changed lines. If you prefer me creating a patch without that bug, I can do > it as well. > > Regards Alberto, thanks for that. I've asked for clarification of bug 823025, which looks like we want it on v1.0.1, but we don't have approval to land there.
After reviewing it, I think is better land this patch first, and not depend on bug 832025 https://github.com/albertopq/gaia/tree/merge-contacts-list-without-823025 823025 patch apply cleanly in top of this, so I think makes more sense in this way. Thanks
Thanks for the merge, Alberto! v1-train: f486e806e61e206186c325b3e065e44d59988339 v1.0.1: d9c7ba71c724de86a5e70a7dee5c353da8bf69ca
No test case is needed in Moztrap for this issue.
Can you please provide steps to verify this fix - as we will blackbox test from the UI?
(In reply to amiller from comment #24) > Can you please provide steps to verify this fix - as we will blackbox test > from the UI? Hi, This was another performance modification, instead of loading ALL js at the beginning of the app we load just the necessary js to display the app and later we do dynamic load of the rest of js. Cheers, F.
Unable to verify performance modifications of the app load. Adding QARegressExclude to the whiteboard
Whiteboard: [FFOS_perf] → [FFOS_perf] QARegressExclude
You need to log in before you can comment on or make changes to this bug.