Closed
Bug 1041291
Opened 10 years ago
Closed 10 years ago
[Vertical Homescreen] Load improvements for configurator.js
Categories
(Firefox OS Graveyard :: Gaia::Homescreen, defect)
Tracking
(blocking-b2g:2.0+, b2g-v2.0 fixed, b2g-v2.1 fixed)
People
(Reporter: kgrandon, Assigned: crdlc)
References
Details
(Whiteboard: [systemsfe])
Attachments
(2 files)
We currently load init.json every time if VersionHelper.isUpgrade returns false. If we already have a configuration saved, we should not need to load and parse this file. My guess is that we could save between ~20-40ms off initial startup.
Reporter | ||
Comment 1•10 years ago
|
||
This that we might be able to improve upon: - Do not always load init.json - Streamline or remove datastore preference access. - Consider caching navigator.getFeature
Summary: [Vertical Homescreen] Do not load init.json once we already have a configuration saved → [Vertical Homescreen] Load improvements for configurator.js
Reporter | ||
Comment 2•10 years ago
|
||
Turns out most of the calls in this file are rather tiny - the bulk of the delay here comes from VersionHelper(). Seems like we should ditch it.
Nominating as per bug 1037706 comment 28.
blocking-b2g: --- → 2.0?
QA Whiteboard: [VH-FL-blocking-][VH-FC-blocking+]
Whiteboard: [systemsfe]
Reporter | ||
Comment 4•10 years ago
|
||
I think we need to profile to determine performance savings amount before we block on this.
blocking-b2g: 2.0? → ---
QA Whiteboard: [VH-FL-blocking-][VH-FC-blocking+] → [VH-FL-blocking-][VH-FC-blocking?]
Assignee | ||
Comment 5•10 years ago
|
||
I gonna try do something here during morning
Assignee: nobody → crdlc
Status: NEW → ASSIGNED
Assignee | ||
Comment 6•10 years ago
|
||
Hi Kevin, the purpose of this patch is to reduce the startup time. The idea is not to load "icc_helper.js", "shared/js/version_helper.js" and "js/configurator.js" when the home has been initialized. Can you take some measure here? I think that we have obtained almost 1 second. My hamachi is much faster :)
Attachment #8460110 -
Flags: feedback?(kgrandon)
Assignee | ||
Updated•10 years ago
|
Attachment #8460110 -
Attachment description: WIP → Github pull request
Attachment #8460110 -
Flags: review?(kgrandon)
Attachment #8460110 -
Flags: review?(carmen.jimenezcabezas)
Attachment #8460110 -
Flags: feedback?(kgrandon)
Reporter | ||
Comment 7•10 years ago
|
||
Comment on attachment 8460110 [details]
Configuration, version helper and SV only for first time
I'm currently measuring to access impact, but the code seems solid to me and should provide a nice boost. I would also like to confirm with Carmen that SV customization should continue to work. Thanks!
Attachment #8460110 -
Flags: review?(kgrandon) → review+
Comment 9•10 years ago
|
||
(In reply to Lucas Adamski [:ladamski] (plz needinfo) from comment #8) > This is a dependency of a CAF bug but isn't blocking 2.0. Should it be? Kevin - What do you think?
Flags: needinfo?(kgrandon)
Reporter | ||
Comment 10•10 years ago
|
||
[Blocking Requested - why for this release]: (In reply to Jason Smith [:jsmith] from comment #9) > (In reply to Lucas Adamski [:ladamski] (plz needinfo) from comment #8) > > This is a dependency of a CAF bug but isn't blocking 2.0. Should it be? > > Kevin - What do you think? I don't think we can make blocking decisions until we get a proper measurement here. I think our options are to either unblock until we get a measurement, or if it would make people feel better, let's just block and assume this will provide a slight boost. It looks like this should be good though and I'll do some measurements tomorrow. Feel free to block for now, thanks.
blocking-b2g: --- → 2.0?
Flags: needinfo?(kgrandon)
Reporter | ||
Comment 11•10 years ago
|
||
I measured and this appears to save us around 200ms. It's worth taking that for 2.0.
Assignee | ||
Comment 12•10 years ago
|
||
1) Delay the drag-drop module after rendering icons and load it lazily 2) Move "sort" method in item.js to "save" method that is only used the first time so we can avoid to parse it next times These are some ideas that don't hurt us and we can gain some milliseconds thought
Attachment #8460787 -
Flags: feedback?(kgrandon)
Reporter | ||
Comment 13•10 years ago
|
||
Comment on attachment 8460787 [details]
Dragdrop feature loaded lazily
I'm confused about moving the save method - I guess the idea is that this moves performance from parse-time to runtime? I'm not really sure if it's worth the readability hit, we should measure the impact of it I think?
Happy about the dragdrop changes, we could also lazy load it inside the collection app I guess.
Attachment #8460787 -
Flags: feedback?(kgrandon) → feedback+
Assignee | ||
Comment 14•10 years ago
|
||
(In reply to Kevin Grandon :kgrandon from comment #13) > Comment on attachment 8460787 [details] > Just another idea > > I'm confused about moving the save method - I guess the idea is that this > moves performance from parse-time to runtime? I'm not really sure if it's > worth the readability hit, we should measure the impact of it I think? I answered in github > > Happy about the dragdrop changes, we could also lazy load it inside the > collection app I guess. Yes, it is done in the pr
Assignee | ||
Updated•10 years ago
|
Attachment #8460787 -
Attachment description: Just another idea → Dragdrop feature loaded lazily
Attachment #8460787 -
Flags: review?(kgrandon)
Assignee | ||
Updated•10 years ago
|
Attachment #8460110 -
Attachment description: Github pull request → Configuration, version helper and SV only for first time
Reporter | ||
Comment 15•10 years ago
|
||
Comment on attachment 8460787 [details]
Dragdrop feature loaded lazily
Looks good to me, can you be sure to fix the unit tests before landing? Thanks!
Attachment #8460787 -
Flags: review?(kgrandon) → review+
Comment 16•10 years ago
|
||
Comment on attachment 8460110 [details]
Configuration, version helper and SV only for first time
I've tested this with and without single variant and with and without a valid SIM present on first run.
It works perfectly, and the patch looks good to me.
Attachment #8460110 -
Flags: review?(carmen.jimenezcabezas) → review+
Updated•10 years ago
|
blocking-b2g: 2.0? → 2.0+
Updated•10 years ago
|
Target Milestone: --- → 2.1 S1 (1aug)
Updated•10 years ago
|
QA Whiteboard: [VH-FL-blocking-][VH-FC-blocking?] → [VH-FL-blocking-][VH-FC-blocking+]
Assignee | ||
Comment 17•10 years ago
|
||
First patch (Configuration, version helper and SV only for first time) merged in master: https://github.com/crdlc/gaia/commit/bb5e48c0536703b89ef8020bd5bb18ca38283301
Assignee | ||
Updated•10 years ago
|
status-b2g-v2.0:
--- → affected
status-b2g-v2.1:
--- → affected
Assignee | ||
Comment 18•10 years ago
|
||
Second patch (Load drag-drop lazily) merged in master: https://github.com/crdlc/gaia/commit/d96402f14f4e17659e9a8126c0f17f944f6e9112
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 19•10 years ago
|
||
v2.0: https://github.com/mozilla-b2g/gaia/commit/d1437e4c4246a7e8c7e16001f4d6bd7d61e9da60 https://github.com/mozilla-b2g/gaia/commit/abdb110c6421c53221ab2d59ddb536bfc062e7a8
Comment 20•10 years ago
|
||
Kevin, do we have the updated measurements here which you were planning to do to see how much improvement the patch gives in 2.0 ?
Flags: needinfo?(kgrandon)
Reporter | ||
Comment 21•10 years ago
|
||
(In reply to bhavana bajaj [:bajaj] from comment #20) > Kevin, do we have the updated measurements here which you were planning to > do to see how much improvement the patch gives in 2.0 ? This patch is already in 2.0. In comment 11 we measured the savings as ~200ms.
Flags: needinfo?(kgrandon)
You need to log in
before you can comment on or make changes to this bug.
Description
•