Closed Bug 837671 Opened 7 years ago Closed 6 years ago
[calendar] Implement new startup loading events
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 Calendar, it should be triggered when the user can actually manipulate the calendar (ie any touch events have been registered)
Summary: "ready to use" perf measurement → [calendar] "ready to use" perf measurement
Bug 996038 introduces new events outlining the phases of application startup. Each of these 5 events needs to be implemented.
Summary: [calendar] "ready to use" perf measurement → [calendar] Implement new startup loading events
Whiteboard: [c=automation p= s= u=] → [c=automation p= s= u=][priority]
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: nobody → gaye
Target Milestone: --- → 2.0 S5 (4july)
I'll take this since Gareth is already working on other tasks this week. Will try to submit a patch tomorrow for review.
Assignee: gaye → mmedeiros
I'm almost sure this is measuring the right things, but please double check it. it took me a while to realize that app was not really ready after the view was loaded, that it really needed the "pending manager" to complete the sync/db operations. Thanks.
Comment on attachment 8445620 [details] [review] Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/20948 So I think we're definitely on the right track and I am convinced that (1) and (2) are basically correct. I think that the description of (3), (4), and (5) imply that the busytimes need to be available in the month view and month day view and I don't think we have that here [yet]. I think accomplishing that bit may be a bit more complicated (though I still think it's worthwhile of course). When we get the `expandComplete` ping from the RecurringEvents controller, we know that https://github.com/millermedeiros/gaia/blob/master/apps/calendar/js/provider/caldav_pull_events.js#L210 has been called for the busytimes in the initial time range. But between that and where https://github.com/millermedeiros/gaia/blob/master/apps/calendar/js/views/day_based.js#L371 gets invoked, there are a few levels of busytime/time observer event hops. Even after that, the time controller goes to the database to find the busytimes belonging to the slice that the view is subscribed https://github.com/millermedeiros/gaia/blob/master/apps/calendar/js/controllers/time.js#L572. Only then does the dom representation of the busytime actually end up in front of the user https://github.com/millermedeiros/gaia/blob/master/apps/calendar/js/views/day_based.js#L421.
So I actually think that this https://github.com/millermedeiros/gaia/blob/master/apps/calendar/js/views/day_based.js#L150 might be a good place to note when its called first that the busytimes have been loaded. Of course we also need to do the same thing for the month view which has diverged a bit. I think that this might be a good place to note that https://github.com/millermedeiros/gaia/blob/master/apps/calendar/js/views/month_child.js#L77.
Review granted from the perf standpoint that the whitelisting is good and names are correct, and from an outsider's POV, look like they would be accurate. I will leave the final location implementation up to Gareth's review.
Comment on attachment 8445620 [details] [review] Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/20948 Gareth, any updates?
Comment on attachment 8445620 [details] [review] Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/20948 Aside from a couple nits, this looks great to me. Thanks Miller!
Attachment #8445620 - Flags: review?(gaye) → review+
landed into master: https://github.com/mozilla-b2g/gaia/commit/d3f97eb1d9197ac071c71e2bf5e3f1097b513412
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
As mentioned in comment 2, this needs to be uplifted to 2.0.
Comment on attachment 8445620 [details] [review] Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/20948 Requesting uplift to 2.0 as it is important for meeting our release performance acceptance criteria. [Feature/regressing bug #]: bug 996038 [User impact if declined]: none [Describe test coverage new/current, TBPL]: Feature only triggers events for testing, no user-facing features or tests [Risks and why]: Low, as there are no user-perceived changes [String/UUID change made/needed]: n/a
Attachment #8445620 - Flags: approval-gaia-v2.0?
(In reply to Eli Perelman, :Eli from comment #12) > Comment on attachment 8445620 [details] [review] > Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/20948 > > Requesting uplift to 2.0 as it is important for meeting our release > performance acceptance criteria. Can you describe how exactly this will help you all? I don't think there are any great risks here either, but would like to understand what material difference this makes for the perf team
Sure thing. On Datazilla, we are running performance metrics against v2.0 as well as master. We cannot give performance sign-off on an application for v2.0 if the means to test that application in that branch are not available. https://wiki.mozilla.org/FirefoxOS/Performance/Release_Acceptance
Ok makes sense. Thanks Eli!
Whiteboard: [c=automation p= s= u=][priority] → [c=automation p= s=2014.07.18.t u=] [priority]
Attachment #8445620 - Flags: approval-gaia-v2.0? → approval-gaia-v2.0+
You need to log in before you can comment on or make changes to this bug.