Closed
Bug 1024014
Opened 10 years ago
Closed 10 years ago
[Marketplace] Implement new startup loading events
Categories
(Marketplace Graveyard :: Code Quality, defect, P1)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: clouserw, Assigned: kngo)
References
Details
(Keywords: perf, Whiteboard: [c=automation p= s= u=])
There is a fine MDN page about app responsiveness guidelines[1] and includes 5 events which we can fire at specific times to automatically track our launch performance in datazilla[2].
This bug is to implement those 5 events in our app. The MDN page refers to a PerformanceTestingHelper script[3]. Harald tells me those scripts are copied into app packages at build times and not simply included off the device.
See bug 996038 for more details about these events and it's dependencies for a bunch of friendly apps trying to implement the same things.
[1] https://developer.mozilla.org/en-US/Apps/Build/Performance/Firefox_OS_app_responsiveness_guidelines
[2] https://datazilla.mozilla.org/b2g/?branch=v1.3t&device=tarako&range=7&test=cold_load_time&app_list=marketplace,phone&app=phone&gaia_rev=2765ccc9bad41f7f&gecko_rev=55ec4880a709&plot=avg
[3] https://github.com/mozilla-b2g/gaia/blob/master/shared/js/performance_testing_helper.js
Updated•10 years ago
|
Reporter | ||
Updated•10 years ago
|
Priority: P2 → --
Comment 1•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
Comment 2•10 years ago
|
||
Leaving the priority empty per the requirements of the team working on it.
Comment 3•10 years ago
|
||
Paging Wil Clouser:
Per our chat, please take a look at this. Here are Eli and Hub's response to whether Marketplace needs to be a packaged app to support these new launch events:
>> Eli: I'm not sure if it needs to be a "packaged" app, but I know there are
>> 2 parts to this.
>>
>> For the first part, nothing needs to be included to just trigger the events
>> since they are just triggered off the window. The second part is getting
>> those events into Datazilla, which at the moment requires that they include
>> a script from "/shared/js/performance_testing_helper.js" which listens for
>> these events, and re-emits them for when make test-perf can record them.
> Hub: They can copy it over. Or something.
>> Eli: As long as this script is included when make test-perf is running, then
>> the metrics will be output and can be consumed. Whether or not an app needs
>> to be "packaged" for this to happen though I am unsure of. Maybe Hub can
>> answer that portion better.
> Hub: Yup, that's all that is needed.
And .. as requested, here's your ascii art :-)
,,,
i i'
\~;\
\; \
\ ;\ ====
\ ;\ ==== \
__,--';;;\-' ( 0
__,--';;; ;;; ;;\ >
__,--'\\ ;;; ;;; ;;; ;;;\--__<
_ _,--' __,--'\\ ;;; __,~~' \ ;\
(_)|_,--' __,--'\\;,~~' \ ;\
|(_)|_,--' ~~ \; \
|| | \ ;\
|_/ !~!,
.---'''---.
| |
| |
| |
`---------'
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → kngo
Reporter | ||
Comment 5•10 years ago
|
||
Kevin (thanks Kevin!) has a patch for this at https://github.com/mozilla/fireplace/pull/490 but we could use a review of where the events are firing. It sounds like the first 4 are all firing at the same time for us due to our splash screen. If IRC or vidyo is easier than bugzilla please say so and we can get together for 10min. :) We're also in #marketplace.
Thanks
Flags: needinfo?(clouserw) → needinfo?(eperelman)
Comment 6•10 years ago
|
||
It's an interesting question.
It's Eli's call, but one option would be to assume that the splash screen is an artifact and fire events when they should be fired (in order to properly measure performance of the boostrap stages of the app) and then adjust the splashscreen to how the events fit in the quotas.
For example, once moz-chrome-dom-loaded is loaded below 1.0s we could stop using the splashscreen for chrome, and only load it in the content area until content is ready.
Reporter | ||
Comment 7•10 years ago
|
||
We landed the patch for this today. I'm unsure if it's working and hub and eli are both on PTO. When you get back (tomorrow, I hear) lets get in touch about comment 5 and overall to see if it's working.
Reporter | ||
Updated•10 years ago
|
Priority: -- → P1
Comment 8•10 years ago
|
||
I'm inclined to think that :gandalf is right here. If we are just firing all the events as soon as the splash screen is gone and it has no direct correlation between when these interactions actually occur, then it won't be very useful in pinpointing regressions in performance, or other platform optimizations either.
That being said, I'm not familiar with this Fireplace app and its UI, so a more detailed explanation of what the UI looks like at each of these event stages would be helpful in understanding the flow.
Flags: needinfo?(eperelman)
Assignee | ||
Comment 9•10 years ago
|
||
Are the events more for performance or for "responsiveness"? Having all of the events fire when our splash screen is gone is appropriate for responsiveness. But if we're testing for performance, then we should change to fire as stuff loads in the background.
Comment 10•10 years ago
|
||
The events were originally conceived for the purposes of performance testing, but there are planned concepts for using these events as indicators for the platform to do better optimizations of painting, among other things. Hence the need for these events to actually be triggered in the correct place at the correct time when the interaction occurs.
For example, if we bind some events necessary to the user to interact with the app, we should fire the event immediately after all these bindings occur. If we end up firing them 100ms later, then yes, technically the user can interact with the elements, but we would not be providing useful data for metrics and platform indicators, which could eventually cause other performance degradations in the app, which is unacceptable.
TLDR, we need to trigger the events when the correct interaction time *actually* occurs, and not some arbitrary point in the future.
Assignee | ||
Comment 11•10 years ago
|
||
It's not arbitrary. In Fireplace, we put up a loading screen which the user cannot interact with until it is hidden. The loading screen is only hidden once everything is loaded (below-the-fold and above-the-fold data). So according to your TLDR and the responsiveness guidelines, the correct user interaction time is when the loading screen is hidden. At that point, we fire 4 out of the 5 events since everything is loaded at that point except for some images.
Is that correct? Or should we fire the events assuming the loading screen is not blocking the user.
Comment 12•10 years ago
|
||
Hmm, if you can answer these few questions that should help me make a determination:
1. Does the app have any chrome (e.g. navigation, headers) apart from the main experience of the app?
2. Not considering the splash screen, do `moz-app-visually-complete` and `moz-content-interactive` actually occur at the same time, e.g. the content and events are injected together?
3. Is the below-the-fold content rendered and injected at a different time than the above-the-fold content?
4. Is there anything else that occurs once the app has loaded, not counting anything that occurs from user interaction?
Assignee | ||
Comment 13•10 years ago
|
||
1. Yes, the app has chrome.
2. Content + events occur at the same time since the app has event delegation.
3. Yes.
4. Yes, we have asynchronous data and lazy-loaded images coming in.
Comment 14•10 years ago
|
||
If we'd want to remove the splashcreen, can we load app's chrome within performance quotas and only need splashcreen for data?
Comment 15•10 years ago
|
||
OK, thinking about this, here is what I would recommend:
1. Trigger the events of `moz-chrome-interactive` and `moz-content-interactive` after the events are actually bound, not waiting for the splash screen to complete.
2. Trigger `moz-app-visually-complete` and `moz-chrome-dom-loaded` when the splash screen is gone and the user can the elements.
3. Trigger `moz-app-loaded` once all async data and image lazy-loading is done.
Assignee | ||
Comment 16•10 years ago
|
||
Thanks for the recommendations, Eli!
https://github.com/mozilla/fireplace/commit/bb7c1e7668e72e136b954d6e9b8ce30fc45c5e98
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•