Closed
Bug 837677
Opened 12 years ago
Closed 10 years ago
[e-mail] Implement new startup loading events
Categories
(Firefox OS Graveyard :: Gaia::E-Mail, defect, P2)
Tracking
(blocking-b2g:-, b2g-v2.0 fixed, b2g-v2.1 fixed)
People
(Reporter: julienw, Assigned: jrburke)
References
Details
(Keywords: perf, Whiteboard: [c=automation p= s= u=][priority])
Attachments
(1 file, 2 obsolete files)
(deleted),
text/x-github-pull-request
|
mcav
:
review+
Eli
:
feedback+
bajaj
:
approval-gaia-v2.0+
|
Details |
We need to measure when the app is usable by the user. For that we'll need to send an event (the moment is specific to the app) to |window| that the performance test will be able to receive.
The event name can be "x-moz-perf-user-ready" amongst all apps so that the performance test can be similar.
For the Email app, it should be triggered when the user can actually manipulate the app.
We should measure 2 runs (cold and warm) for this app:
- in the first run we'll measure when we arrive to the "set up an account" page and can manipulate it.
- in the second run an account should be set up in advance, and we'll measure when the user can manipulate the display of the mails that are already stored locally.
Reporter | ||
Updated•12 years ago
|
Blocks: gaia-perf-measure
No longer depends on: gaia-perf-measure
Reporter | ||
Updated•12 years ago
|
Summary: "ready to use" perf measurement → [e-mail] "ready to use" perf measurement
Assignee | ||
Comment 1•12 years ago
|
||
I am willing to add this for the email app. From what I can tell, it means emitting an event in mail-app.js in "showMessageViewOrSetup" after the final card pushes, and then testing it out to make sure it lines up to the user seeing something, and if not, adjusting the location of the event being emitted to match.
Julien, if you think it needs more than this, or if you were going to look into it, please feel free to let me know.
Flags: needinfo?(felash)
Reporter | ||
Comment 2•12 years ago
|
||
James, I refined some code in Bug 837673 (patch ready in r?) so I'd suggest you take a look there and even |git am| the patch on your local tree so that you can use the shared objects that I made.
Basically you can emit different events at different moments if necessary (but if only one is necessary then just emit one of course). The important thing is, as you said, to find the correct moment. For me, it is the moment when the user can start interacting with the app (so for example he can open a mail).
In the future this may need refining, with or without network, etc, but for now we can start with a first simple measurement.
Flags: needinfo?(felash)
Reporter | ||
Comment 3•12 years ago
|
||
also, so that you don't lose time as I did ;) you need a very recent b2g18 tree with the fix to Bug 842841.
Assignee | ||
Comment 4•12 years ago
|
||
Pointer to Github pull-request
Assignee | ||
Comment 5•12 years ago
|
||
Comment on attachment 745230 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/9544
This wires up the PerformanceHelper to email. However it also includes some changes to the performance helper code.
The executeAsyncScript code to register the mozPerfHasListener was getting stuck in the event loop after the places in the code where PerformanceTestingHelper.dispatch() was called in the email app. I even saw this occasionally in the communications/contacts app.
So I modified shared/js/performance_testing_helper.js to store on to the dispatch data and check periodically for mozPerfHasListener. The dispatch data also uses a time offset relative to navigation.performance.fetchStart, which is closer to when the current app starts loading.
Since the event data may not be sent until later, I modified tests/performance/performance_helper_atom.js to use the timestamp in the data instead of calling performance.now() manually.
Asking julienw for a review of the performance helper changes, asuth for mail measuring points.
Attachment #745230 -
Flags: review?(felash)
Attachment #745230 -
Flags: review?(bugmail)
Reporter | ||
Comment 6•12 years ago
|
||
Rik, if you find yourself having some free time, please go ahead and steal this review from me.
Flags: needinfo?(anthony)
Assignee | ||
Comment 7•12 years ago
|
||
Pointer to Github pull-request
Updated•12 years ago
|
Attachment #745325 -
Attachment is obsolete: true
Updated•12 years ago
|
Attachment #745230 -
Flags: review?(felash)
Comment 9•11 years ago
|
||
Comment on attachment 745230 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/9544
(This patched was mooted by rik's review comment that cited bug 846909 as the right place to address this. This but should potentially be resolved as invalid.)
Attachment #745230 -
Flags: review?(bugmail)
Updated•11 years ago
|
Priority: -- → P2
Updated•11 years ago
|
Whiteboard: [c=instrumentation p=] → [c=automation p= s= u=]
Updated•11 years ago
|
Blocks: gaia-perf-events
Comment 10•11 years ago
|
||
Bug 996038 introduces new events outlining the phases of application startup. Each of these 5 events needs to be implemented.
Summary: [e-mail] "ready to use" perf measurement → [e-mail] Implement new startup loading events
Updated•10 years ago
|
Whiteboard: [c=automation p= s= u=] → [c=automation p= s= u=][priority]
Updated•10 years ago
|
Assignee: nobody → awissmann
Updated•10 years ago
|
Assignee: awissmann → nobody
Comment 11•10 years ago
|
||
As an FYI, this implementation needs to land in 2.0 as it is important for meeting release performance acceptance criteria.
https://wiki.mozilla.org/FirefoxOS/Performance/Release_Acceptance
Assignee | ||
Comment 12•10 years ago
|
||
Pull request, asking :mcav since :asuth will be in an afk window soon. Also asking :Eli for feedback, although he will also be afk for a bit, so will not hold up the pull request if r+ lands while in the afk window.
Attachment #745230 -
Attachment is obsolete: true
Attachment #8439582 -
Flags: review?(m)
Attachment #8439582 -
Flags: feedback?(eperelman)
Comment 13•10 years ago
|
||
Comment on attachment 8439582 [details]
GitHub pull request
Quite the PR, and everything is on the up-and-up for the performance side. Looks good.
Attachment #8439582 -
Flags: feedback?(eperelman) → feedback+
Comment 14•10 years ago
|
||
Comment on attachment 8439582 [details]
GitHub pull request
Looks good, the comments are clear.
Attachment #8439582 -
Flags: review?(m) → review+
Assignee | ||
Comment 15•10 years ago
|
||
Merged in master:
https://github.com/mozilla-b2g/gaia/commit/f93e6fa38b3f328df82a493406c8ec52a890c2c5
from pull request:
https://github.com/mozilla-b2g/gaia/pull/20464
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Updated•10 years ago
|
Target Milestone: --- → 2.0 S4 (20june)
Comment 16•10 years ago
|
||
James, could you mark your patch for uplift to 2.0? This implementation needs to land in 2.0 as it is important for meeting release performance acceptance criteria.
https://wiki.mozilla.org/FirefoxOS/Performance/Release_Acceptance
Updated•10 years ago
|
Flags: needinfo?(jrburke)
Assignee | ||
Comment 17•10 years ago
|
||
Asking for 2.0? as per comment 16. It may not uplift cleanly via automation, due to a possible conflict in compose.js, but if so, just ping me and I will manually prep the patch.
blocking-b2g: --- → 2.0?
Flags: needinfo?(jrburke)
Comment 18•10 years ago
|
||
(In reply to James Burke [:jrburke] from comment #17)
> Asking for 2.0? as per comment 16. It may not uplift cleanly via automation,
> due to a possible conflict in compose.js, but if so, just ping me and I will
> manually prep the patch.
This should go through approval, it seems like we are trying to add new measurements to better get perf results, not something we would block a release on. Feel free to request approval-gaia-v2.0 on this.
blocking-b2g: 2.0? → -
Assignee | ||
Comment 19•10 years ago
|
||
Comment on attachment 8439582 [details]
GitHub pull request
Filing on behalf of :Eli, but if there are follow up questions on the importance of this landing in 2.0, :Eli should field those questions.
[Bug caused by] (feature/regressing bug #):
none, feature request by the performance team. See comment #16, measuring startup events part of acceptance criteria.
[User impact] if declined:
none
[Testing completed]:
Running the perf commands to collect the metrics.
[Risk to taking this patch] (and alternatives if risky):
Low, just emits some events that are tracked by performance tools.
[String changes made]:
none
Attachment #8439582 -
Flags: approval-gaia-v2.0?
Updated•10 years ago
|
Attachment #8439582 -
Flags: approval-gaia-v2.0? → approval-gaia-v2.0+
Comment 20•10 years ago
|
||
status-b2g-v2.0:
--- → fixed
status-b2g-v2.1:
--- → fixed
Updated•10 years ago
|
Assignee: nobody → jrburke
You need to log in
before you can comment on or make changes to this bug.
Description
•