1356871 - Add timing related columns

Status: RESOLVED FIXED

(DevTools :: Netmonitor, enhancement, P3)

Description

[Tim Nguyen :ntim]

Safari has:
- latency
- duration
- start time

Chrome has:
- start time
- end time
- response time
- duration
- latency

Comment 1

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Thanks for reporting this. I think we can have the same as Chrome.

Honza

Comment 2

[Vangelis Katsikaros]

Attachment: sorting.png

Would you like each timing (start time, end time, etc) to be a column on its own or a sorting on the waterfall column? 

On Chrome, there is a separate column "Time" that is the "Total Time" ("duration" from the timings mentioned above).

Comment 3

[Tim Nguyen :ntim]

(In reply to Vangelis Katsikaros from comment #2)
> Created attachment 8863426 [details]
> sorting.png
> 
> Would you like each timing (start time, end time, etc) to be a column on its
> own or a sorting on the waterfall column? 

I personally find chrome's UX confusing, I prefer we have 1 column for each timing.

Comment 4

[Vangelis Katsikaros]

I would like to ask, if the following are the correct definitions for the above columns:
- start time: from start of 1st request until the start of this request
- end time: from start of 1st request until the end of this request
- duration: from start of this request until the end of this request 
- latency: from of start this request until the beginning of download of this request 
- response time: from finished sending this request until the beginning of download of this request (ie eventTimings.timings.wait)

Comment 5

[Tim Nguyen :ntim]

(In reply to Vangelis Katsikaros from comment #4)
> I would like to ask, if the following are the correct definitions for the
> above columns:
> - start time: from start of 1st request until the start of this request
Yes.
> - end time: from start of 1st request until the end of this request
> - duration: from start of this request until the end of this request
> - latency: from of start this request until the beginning of download of
> this request 
> - response time: from finished sending this request until the beginning of
> download of this request (ie eventTimings.timings.wait)

These definitions seem right to me.

But Honza might be a better person to check with.

Comment 6

[Tim Nguyen :ntim]

Here's how Chrome defines these terms: https://developers.google.com/web/tools/chrome-devtools/network-performance/reference#sort-by-activity

Comment 7

[Jan Honza Odvarko [:Honza] (always need-info? me)]

(In reply to Vangelis Katsikaros from comment #4)
> I would like to ask, if the following are the correct definitions for the
> above columns:
> - start time: from start of 1st request until the start of this request
Yes

> - end time: from start of 1st request until the end of this request
Yes, where 'end' means the response body is fully downloaded.

> - duration: from start of this request until the end of this request 
Yes (again including the response body downloading phase)

> - latency: from of start this request until the beginning of download of
> this request 
I think that it's from end of sending phase (finished sending of this request)
until the beginning of download of this request.

In another words (Chrome docs): The request that waited the shortest time for a response is at the top.

I think this is: eventTimings.timings.wait

> - response time: from finished sending this request until the beginning of
> download of this request (ie eventTimings.timings.wait)
I think it's time from start of 1st request until the beginning of
download of this request.

In another words (Chrome docs) - the first request that started downloading is at the top.

---

So, I think you mixed Latency & Response time

Honza

Comment 8

[Jan Honza Odvarko [:Honza] (always need-info? me)]

(In reply to Tim Nguyen :ntim from comment #3)
> (In reply to Vangelis Katsikaros from comment #2)
> > Created attachment 8863426 [details]
> > sorting.png
> > 
> > Would you like each timing (start time, end time, etc) to be a column on its
> > own or a sorting on the waterfall column? 
> 
> I personally find chrome's UX confusing, I prefer we have 1 column for each
> timing.
Yes, I agree. Individual columns are better.

Honza

Comment 9

[Vangelis Katsikaros]

Attachment: Bug 1356871 - Add timing related columns.

Review commit: https://reviewboard.mozilla.org/r/136772/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/136772/

Comment 10

[Vangelis Katsikaros]

Attachment: new_columns.png

Demo of the new columns

Comment 11

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Comment on attachment 8865107 [details]
Bug 1356871 - Add timing related columns.

https://reviewboard.mozilla.org/r/136772/#review139962

Excellent!

Couple of comments:

1) The calculation of the response time seems to be wrong. It doesn't include waiting time. I'll attach a screenshot.
2) Can you put all the new context menu actions (related to timing) into a submenu labeled as "Timings"?


Thanks for working on this!
Honza

Comment 12

[Jan Honza Odvarko [:Honza] (always need-info? me)]

(In reply to Jan Honza Odvarko [:Honza] from comment #11)
> 1) The calculation of the response time seems to be wrong. It doesn't
> include waiting time. I'll attach a screenshot.
Sorry, I double checked and it seems to be ok :-)

The waiting time is properly included and the 'receiving' time is properly *not* included

Honza

Comment 13

[Vangelis Katsikaros]

Comment on attachment 8865107 [details]
Bug 1356871 - Add timing related columns.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/136772/diff/1-2/

Comment 14

[Vangelis Katsikaros]

Attachment: timings-submenu.png

Comment 15

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Comment on attachment 8865107 [details]
Bug 1356871 - Add timing related columns.

https://reviewboard.mozilla.org/r/136772/#review142526

Looks great, thanks for the patch!

r+ assuming try is green

Honza

Comment 16

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Btw. the patch needs to be rebased on latest head.

Honza

Comment 17

[Vangelis Katsikaros]

Comment on attachment 8865107 [details]
Bug 1356871 - Add timing related columns.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/136772/diff/2-3/

Comment 18

[Vangelis Katsikaros]

Thanks for the reviews Honza, I rebased it on the latest head (358609:41958333867b)

However, I see some tests failing on try due to timeouts. I don't have a MacOS (the try's failure) to run the test locally, and I am not sure I can see what is wrong from the output when clicking at a failure.

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7d07aa43577e
Add timing related columns. r=Honza

Comment 20

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/7d07aa43577e

Comment 21

[Saheda Reza [:Antora]]

I have reproduced this on Firefox nightly according to (2017-04-16)

Fixing bug is verified on Latest Nightly-- Build ID:(20170602030204), User Agent: Mozilla/5.0 (Windows NT 6.1; rv:55.0) Gecko/20100101 Firefox/55.0

Tested OS--Windows7 32bit

[bugday-20170531]