[Loop] Performance logger

RESOLVED FIXED

Status

Firefox OS
Gaia::Loop
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: ferjm, Assigned: ferjm)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 2 obsolete attachments)

Comment hidden (empty)
(Assignee)

Updated

4 years ago
Assignee: nobody → ferjmoreno
(Assignee)

Comment 1

4 years ago
Created attachment 8485878 [details]
v1
Attachment #8485878 - Flags: feedback?(josea.olivera)
Attachment #8485878 - Flags: feedback?(dcoloma)
Attachment #8485878 - Flags: feedback?(borja.bugzilla)
(Assignee)

Updated

4 years ago
Attachment #8485879 - Attachment is obsolete: true
(Assignee)

Comment 3

4 years ago
Created attachment 8485881 [details]
Log example
(Assignee)

Comment 4

4 years ago
Created attachment 8485882 [details]
Log example
Attachment #8485881 - Attachment is obsolete: true
(Assignee)

Comment 5

4 years ago
The attached patch adds a helper that allow us to log events and times to help us measure the performance of the Loop app.

The logs can be dumped in the console and also be saved in a text file in the sdcard. Attached is also an example of a perf log saved in the sdcard.

The API for the logger is quite simple:

- startTracing: starts logging events for the specified functionality.
- stopTracing: stops logging events for that functionality and dumps the already logged events in the console and (optionally) to the disk.
- log: logs an event and the times associated with it.
- milestone: sets a milestone so the next logged events timing can be calculated with the milestone as a reference. We can add as many milestones as we want for each functionality being logged.
(Assignee)

Updated

4 years ago
Attachment #8485878 - Flags: feedback?(dcoloma)
(Assignee)

Updated

4 years ago
Attachment #8485882 - Flags: feedback?(dcoloma)
Comment on attachment 8485882 [details]
Log example

The log looks awesome! the only comment I have is that we are requesting an extra permission (sdcard). Is the use of that permission optional (i.e. if end-users reject it everything continue working as expected)?
Comment on attachment 8485882 [details]
Log example

As Maria said, this looks great! I assume eventually we could also send part of this information to telemetry to get aggregated information about the quality of the establishment process, but that is an extra on top of this.
Attachment #8485882 - Flags: feedback?(dcoloma) → feedback+
(Assignee)

Comment 8

4 years ago
(In reply to Maria Angeles Oteo (:oteo) from comment #6)
> Comment on attachment 8485882 [details]
> Log example
> 
> The log looks awesome! the only comment I have is that we are requesting an
> extra permission (sdcard). Is the use of that permission optional (i.e. if
> end-users reject it everything continue working as expected)?

Yes, this is optional and in any case it won't be requested on production. This is just to help us debug performance issues :). Also, even if we don't write the report in the sdcard, we dump it to the console.
(Assignee)

Comment 9

4 years ago
(In reply to Daniel Coloma:dcoloma from comment #7)
> Comment on attachment 8485882 [details]
> Log example
> 
> As Maria said, this looks great! I assume eventually we could also send part
> of this information to telemetry to get aggregated information about the
> quality of the establishment process, but that is an extra on top of this.

Yes, we could add this timing measures to telemetry if needed.
(Assignee)

Comment 10

4 years ago
Comment on attachment 8485878 [details]
v1

This patch introduces the performance logger and a trace for the call screen. Unfortunately, I couldn't manage to get the exact moment when the screen shows both local and remote videos to stop getting the traces. I tried with all the events from [1], but these are triggered a few seconds (between 2 and 5 secs) before there is actually a video on the screen. There is also a race condition where we may add the event listeners for the remote video element too late. This means that sometimes we don't log the moment when we receive the event about the remote video having enough data to be played. The only way that I can think about to fix this is to ask to TokBox to expose this media events in their API, but I don't think is worth the effort as the events are not really showing the reality.

[1] https://developer.mozilla.org/en-US/docs/Web/Guide/Events/Media_events
Attachment #8485878 - Attachment description: WIP → v1
Attachment #8485878 - Flags: review?(josea.olivera)
Attachment #8485878 - Flags: review?(borja.bugzilla)
Attachment #8485878 - Flags: feedback?(josea.olivera)
Attachment #8485878 - Flags: feedback?(borja.bugzilla)
Comment on attachment 8485878 [details]
v1

LGTM. r=me

Sorry for the lag. Thanks!
Attachment #8485878 - Flags: review?(josea.olivera) → review+
Attachment #8485878 - Flags: review?(borja.bugzilla) → review+
(Assignee)

Comment 12

4 years ago
https://github.com/mozilla-b2g/firefoxos-loop-client/commit/450bdbb0ced35ab2b40672e4e72203d577330442
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.