Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2057243003:
DevTools: [network] Show request start time in timing table (Closed)
|
Created:
4 years, 6 months ago by paulirish Modified:
3 years, 10 months ago CC:
apavlov+blink_chromium.org, blink-reviews, caseq+blink_chromium.org, chromium-reviews, devtools-reviews_chromium.org, kozyatinskiy+blink_chromium.org, lushnikov+blink_chromium.org, pfeldman+blink_chromium.org, pfeldman, sergeyv+blink_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionDevTools: [network] Show request start time in timing table
Screenshot: http://i.imgur.com/hlPOiWZ.png
This CL picks up from https://codereview.chromium.org/1232733002
BUG=447200, 476749
Review-Url: https://codereview.chromium.org/2057243003
Cr-Commit-Position: refs/heads/master@{#449808}
Committed: https://chromium.googlesource.com/chromium/src/+/d2b07d2dd7299cd250c2ae15a9b0cfedd2bec5b3
Patch Set 1 #Patch Set 2 : rebase #Patch Set 3 : connection setup -> start #
Total comments: 2
Patch Set 4 : fix lint #Patch Set 5 : Common.UIString #
Messages
Total messages: 30 (17 generated)
Description was changed from ========== DevTools: [network] Show request start time in timing table BUG= ========== to ========== DevTools: [network] Show request start time in timing table Screenshot: http://i.imgur.com/hlPOiWZ.png Also, fixes the -1ms value that occasionally shows up: http://i.imgur.com/LHxYDMA.png The reason we got a -1 is the NetworkPanel's calculator has a minimumBoundary that was defined by the "requestWillBeSent" start timestamp (#1). However once the request is sent, the startTime of the request is updated (#2), however the bounds of the network waterfall were defined by the #2 time, but the request.startTime we have been using is actually the request.issueTime which is #1. As a result, about a millisecond of ambiguous "queueing" time snuck in there. This time is explained by this comment. https://code.google.com/p/chromium/issues/detail?id=476749#c11 BUG=447200,476749 ==========
Description was changed from ========== DevTools: [network] Show request start time in timing table Screenshot: http://i.imgur.com/hlPOiWZ.png Also, fixes the -1ms value that occasionally shows up: http://i.imgur.com/LHxYDMA.png The reason we got a -1 is the NetworkPanel's calculator has a minimumBoundary that was defined by the "requestWillBeSent" start timestamp (#1). However once the request is sent, the startTime of the request is updated (#2), however the bounds of the network waterfall were defined by the #2 time, but the request.startTime we have been using is actually the request.issueTime which is #1. As a result, about a millisecond of ambiguous "queueing" time snuck in there. This time is explained by this comment. https://code.google.com/p/chromium/issues/detail?id=476749#c11 BUG=447200,476749 ========== to ========== DevTools: [network] Show request start time in timing table Screenshot: http://i.imgur.com/hlPOiWZ.png The NetworkPanel's calculator has a minimumBoundary that was defined by the "requestWillBeSent" start timestamp (#1). However once the request is sent, the startTime of the request is updated (#2), however the bounds of the network waterfall were defined by the #2 time, but the request.startTime we have been using is actually the request.issueTime which is #1. As a result, about a millisecond of ambiguous "queueing" time snuck in there. https://code.google.com/p/chromium/issues/detail?id=476749#c11 We fix that in this CL and avoid "-1ms" start times on the main document. BUG=447200,476749 ==========
Description was changed from ========== DevTools: [network] Show request start time in timing table Screenshot: http://i.imgur.com/hlPOiWZ.png The NetworkPanel's calculator has a minimumBoundary that was defined by the "requestWillBeSent" start timestamp (#1). However once the request is sent, the startTime of the request is updated (#2), however the bounds of the network waterfall were defined by the #2 time, but the request.startTime we have been using is actually the request.issueTime which is #1. As a result, about a millisecond of ambiguous "queueing" time snuck in there. https://code.google.com/p/chromium/issues/detail?id=476749#c11 We fix that in this CL and avoid "-1ms" start times on the main document. BUG=447200,476749 ========== to ========== DevTools: [network] Show request start time in timing table Screenshot: http://i.imgur.com/hlPOiWZ.png The NetworkPanel's calculator has a minimumBoundary that was defined by the "requestWillBeSent" start timestamp (#1). However once the request is sent, the startTime of the request is updated (#2), however the bounds of the network waterfall were defined by the #2 time, but the request.startTime we have been using is actually the request.issueTime which is #1. As a result, about a millisecond of ambiguous "queueing" time snuck in there. (https://code.google.com/p/chromium/issues/detail?id=476749#c11) We fix that in this CL and avoid "-1ms" start times on the main document. BUG=447200,476749 ==========
Description was changed from ========== DevTools: [network] Show request start time in timing table Screenshot: http://i.imgur.com/hlPOiWZ.png The NetworkPanel's calculator has a minimumBoundary that was defined by the "requestWillBeSent" start timestamp (#1). However once the request is sent, the startTime of the request is updated (#2), however the bounds of the network waterfall were defined by the #2 time, but the request.startTime we have been using is actually the request.issueTime which is #1. As a result, about a millisecond of ambiguous "queueing" time snuck in there. (https://code.google.com/p/chromium/issues/detail?id=476749#c11) We fix that in this CL and avoid "-1ms" start times on the main document. BUG=447200,476749 ========== to ========== DevTools: [network] Show request start time in timing table Screenshot: http://i.imgur.com/hlPOiWZ.png The NetworkPanel's calculator has a minimumBoundary that was defined by the "requestWillBeSent" start timestamp (#1). However once the request is sent, the startTime of the request is updated (#2), however the bounds of the network waterfall were defined by the #2 time, but the request.startTime we have been using is actually the request.issueTime which is #1. As a result, about a millisecond of ambiguous "queueing" time snuck in there. (https://code.google.com/p/chromium/issues/detail?id=476749#c11) We fix that in this CL and avoid "-1ms" start times on the main document. This CL picks up from https://codereview.chromium.org/1232733002 BUG=447200,476749 ==========
paulirish@chromium.org changed reviewers: + allada@chromium.org, caseq@chromium.org
PTAL
Description was changed from ========== DevTools: [network] Show request start time in timing table Screenshot: http://i.imgur.com/hlPOiWZ.png The NetworkPanel's calculator has a minimumBoundary that was defined by the "requestWillBeSent" start timestamp (#1). However once the request is sent, the startTime of the request is updated (#2), however the bounds of the network waterfall were defined by the #2 time, but the request.startTime we have been using is actually the request.issueTime which is #1. As a result, about a millisecond of ambiguous "queueing" time snuck in there. (https://code.google.com/p/chromium/issues/detail?id=476749#c11) We fix that in this CL and avoid "-1ms" start times on the main document. This CL picks up from https://codereview.chromium.org/1232733002 BUG=447200,476749 ========== to ========== DevTools: [network] Show request start time in timing table Screenshot: http://i.imgur.com/hlPOiWZ.png This CL picks up from https://codereview.chromium.org/1232733002 BUG=447200,476749 ==========
lgtm
lgtm
caseq can you hook me up with a review?
caseq@chromium.org changed reviewers: + chowse@chromium.org
+chowse code lgtm, but per Pavel's suggestion, let's make these visually distinct from the rest (darker background?)
On 2016/08/09 00:09:25, caseq wrote: > +chowse > > code lgtm, but per Pavel's suggestion, let's make these visually distinct from > the rest (darker background?) Can you explain what this CL changes? Is it just the "Queued at 0" and "Started at 12.95ms"? Is "0" supposed to be "0ms"? Are these absolute timeframes?
On 2016/08/09 01:17:48, chowse wrote: > Is "0" supposed to be "0ms"? Are these absolute timeframes? Times are relative to the state of navigation (so first request is always queued at 0). 0 should be dimensionless I think. It's 0ms, yes, but it's also 0s, 0m and 0us.
On 2016/08/09 01:45:56, caseq wrote: > On 2016/08/09 01:17:48, chowse wrote: > > Is "0" supposed to be "0ms"? Are these absolute timeframes? > > Times are relative to the state of navigation (so first request is always queued > at 0). > > 0 should be dimensionless I think. It's 0ms, yes, but it's also 0s, 0m and 0us. My first reaction when I saw it was that is was an ordering-- otherwise, why wouldn't it be "0ms" like all the other values? If it's a measure of time, I recommend keeping it "0ms" for consistency. Regarding how to show the times: if you're feeling adventurous, you could frame the bars inside a mini-timeline. Example (with and w/o start time callout): http://i.imgur.com/KZVXpJA.png
Description was changed from ========== DevTools: [network] Show request start time in timing table Screenshot: http://i.imgur.com/hlPOiWZ.png This CL picks up from https://codereview.chromium.org/1232733002 BUG=447200,476749 ========== to ========== DevTools: [network] Show request start time in timing table Screenshot: http://i.imgur.com/hlPOiWZ.png This CL picks up from https://codereview.chromium.org/1232733002 BUG=447200,476749 ==========
allada@chromium.org changed reviewers: - allada@chromium.org
The CQ bit was checked by paulirish@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from caseq@chromium.org, allada@chromium.org Link to the patchset: https://codereview.chromium.org/2057243003/#ps40001 (title: "connection setup -> start")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
alph@chromium.org changed reviewers: + alph@chromium.org
https://codereview.chromium.org/2057243003/diff/40001/third_party/WebKit/Sour... File third_party/WebKit/Source/devtools/front_end/network/RequestTimingView.js (right): https://codereview.chromium.org/2057243003/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/devtools/front_end/network/RequestTimingView.js:216: queueingHeader.createChild("td").createTextChild("Resource Scheduling"); Common.UIString https://codereview.chromium.org/2057243003/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/devtools/front_end/network/RequestTimingView.js:218: queueingHeader.createChild("td").createTextChild("TIME"); ditto
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by paulirish@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from caseq@chromium.org, allada@chromium.org Link to the patchset: https://codereview.chromium.org/2057243003/#ps80001 (title: "Common.UIString")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 80001, "attempt_start_ts": 1486769248036510,
"parent_rev": "2bc44a555e1b2f4250e752b0ab527470c6ad5c56", "commit_rev":
"d2b07d2dd7299cd250c2ae15a9b0cfedd2bec5b3"}
Message was sent while issue was closed.
Description was changed from ========== DevTools: [network] Show request start time in timing table Screenshot: http://i.imgur.com/hlPOiWZ.png This CL picks up from https://codereview.chromium.org/1232733002 BUG=447200,476749 ========== to ========== DevTools: [network] Show request start time in timing table Screenshot: http://i.imgur.com/hlPOiWZ.png This CL picks up from https://codereview.chromium.org/1232733002 BUG=447200,476749 Review-Url: https://codereview.chromium.org/2057243003 Cr-Commit-Position: refs/heads/master@{#449808} Committed: https://chromium.googlesource.com/chromium/src/+/d2b07d2dd7299cd250c2ae15a9b0... ==========
Message was sent while issue was closed.
Committed patchset #5 (id:80001) as https://chromium.googlesource.com/chromium/src/+/d2b07d2dd7299cd250c2ae15a9b0... |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
