Closed
Bug 835791
Opened 12 years ago
Closed 12 years ago
[Contacts] Improve opening time performance
Categories
(Firefox OS Graveyard :: Gaia::Contacts, defect)
Tracking
(blocking-b2g:tef+, b2g18+ fixed, b2g18-v1.0.1 fixed)
RESOLVED
FIXED
blocking-b2g | tef+ |
People
(Reporter: alberto.pastor, Assigned: alberto.pastor)
References
Details
(Keywords: perf, Whiteboard: [FFOS_perf] QARegressExclude)
Attachments
(1 file, 1 obsolete file)
We need to reduce as much as possible all the content that is initially loaded and is not needed for the initial screen
Assignee | ||
Updated•12 years ago
|
Updated•12 years ago
|
Whiteboard: [FFOS_perf]
Comment 1•12 years ago
|
||
Without a more concrete plan or issue here, we can't block so I'm going to mark is as tracking.
blocking-b2g: tef? → -
tracking-b2g18:
--- → +
Assignee | ||
Comment 2•12 years ago
|
||
Pointer to Github pull-request
Assignee | ||
Updated•12 years ago
|
Attachment #707755 -
Attachment is obsolete: true
Updated•12 years ago
|
blocking-b2g: - → tef?
Comment 4•12 years ago
|
||
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+
Comment 5•12 years ago
|
||
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
status-b2g18:
--- → affected
status-b2g18-v1.0.0:
--- → affected
Assignee | ||
Comment 7•12 years ago
|
||
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.
Flags: needinfo?(alberto.pastor)
Comment 8•12 years ago
|
||
(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!
Assignee | ||
Comment 9•12 years ago
|
||
Attachment #712386 -
Flags: review?(francisco.jordano)
Assignee | ||
Comment 10•12 years ago
|
||
WIth the patch applied
=== Contacts average load time: 1280ms
Comment 11•12 years ago
|
||
Comments made on the code, patch looking good, just waiting for address some changes regarding syncing when fb is completely loaded.
Updated•12 years ago
|
status-b2g18-v1.0.1:
--- → affected
Updated•12 years ago
|
Attachment #712386 -
Flags: review?(crdlc)
Updated•12 years ago
|
Attachment #712386 -
Flags: review?(crdlc) → review+
Comment 12•12 years ago
|
||
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 13•12 years ago
|
||
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+
Assignee | ||
Comment 14•12 years ago
|
||
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Comment 15•12 years ago
|
||
Batch edit: Bugs still affected on b2g18 after 2/13 merge to v1.0.1 branch are affected on v1.0.1 branch.
Comment 16•12 years ago
|
||
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
Comment 17•12 years ago
|
||
Alberto, can you take a look at this ASAP please? If this fix is not uplifted it will not make the release.
Assignee | ||
Comment 18•12 years ago
|
||
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,
Assignee | ||
Comment 19•12 years ago
|
||
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
Comment 20•12 years ago
|
||
(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.
Assignee | ||
Comment 21•12 years ago
|
||
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
Comment 22•12 years ago
|
||
Thanks for the merge, Alberto!
v1-train: f486e806e61e206186c325b3e065e44d59988339
v1.0.1: d9c7ba71c724de86a5e70a7dee5c353da8bf69ca
Comment 24•12 years ago
|
||
Can you please provide steps to verify this fix - as we will blackbox test from the UI?
Comment 25•12 years ago
|
||
(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.
Comment 26•12 years ago
|
||
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.
Description
•