Add Navigation.URLLoader.OnAcceptCHFrameReceived.RestartTime [chromium/src : main]
Hiroshige Hayashizaki (Gerrit)
Hiroshige Hayashizaki voted Commit-Queue+1![]( "Open in Gerrit")
| Commit-Queue | +1 |
Related details
- Kouhei Ueno
- Yoshisato Yanagisawa
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Kouhei Ueno (Gerrit)
Kouhei Ueno voted Code-Review+1![]( "Open in Gerrit")
- Hiroshige Hayashizaki
- Yoshisato Yanagisawa
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Kouhei Ueno (Gerrit)
Kouhei Ueno added 1 comment![]( "Open in Gerrit")
- Hiroshige Hayashizaki
- Jiacheng Guo
- Yoshisato Yanagisawa
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Yoshisato Yanagisawa (Gerrit)
Yoshisato Yanagisawa voted and added 2 comments![]( "Open in Gerrit")
Votes added by Yoshisato Yanagisawa
| Code-Review | +1 |
2 comments
Sorry for the slow reply, I do not want to block your experiment but I did not convince the metrics work.
"Navigation.URLLoader.OnAcceptCHFrameReceived.RestartTime", restart_time);Will this function be called?
I understand the behavior is like:
1. (network) Receives AcceptCHFrame
2. (network) Some required hints are missing and merge headers for restart.
3. (network -> browser) IPC to AcceptCHFrameReceived here.
4. (browser) restarts with fields required by ACCEPT CH frame
5. (network) AcceptCHFrame received.
6. (network) since all required headers exist, continue in the network layer. i.e. AcceptCHFrameReceived won't be called.
Then, Line 1897-1903 might not be called after restart.
What do you think?
Related details
- Hiroshige Hayashizaki
- Jiacheng Guo
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Jiacheng Guo (Gerrit)
Jiacheng Guo added 2 comments![]( "Open in Gerrit")
"Navigation.URLLoader.OnAcceptCHFrameReceived.RestartTime", restart_time);Will this function be called?
I understand the behavior is like:
1. (network) Receives AcceptCHFrame
2. (network) Some required hints are missing and merge headers for restart.
3. (network -> browser) IPC to AcceptCHFrameReceived here.
4. (browser) restarts with fields required by ACCEPT CH frame
5. (network) AcceptCHFrame received.
6. (network) since all required headers exist, continue in the network layer. i.e. AcceptCHFrameReceived won't be called.Then, Line 1897-1903 might not be called after restart.
What do you think?
Checked the behavior of
"Navigation.URLLoader.OnAcceptCHFrameReceived.RestartTime", restart_time);Will this function be called?
I understand the behavior is like:
1. (network) Receives AcceptCHFrame
2. (network) Some required hints are missing and merge headers for restart.
3. (network -> browser) IPC to AcceptCHFrameReceived here.
4. (browser) restarts with fields required by ACCEPT CH frame
5. (network) AcceptCHFrame received.
6. (network) since all required headers exist, continue in the network layer. i.e. AcceptCHFrameReceived won't be called.Then, Line 1897-1903 might not be called after restart.
What do you think?
Agree that this method might not be called after restart if all the client hints are provided. Since the browser destructs the URL loader and creats a new one, it will be quite complicated to pass the restart time to the URL loader and record the latency to receive accept-CH frame again. Is measuring the time between the `NavigationURLLoaderImpl::Restart` and `NavigationURLLoaderImpl::OnReceiveResponse` or `NavigationURLLoaderImpl::OnReceiveRedirect` an acceptable rough estimate?
Related details
- Hiroshige Hayashizaki
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Hiroshige Hayashizaki (Gerrit)
Hiroshige Hayashizaki added 1 comment![]( "Open in Gerrit")
"Navigation.URLLoader.OnAcceptCHFrameReceived.RestartTime", restart_time);Jiacheng GuoWill this function be called?
I understand the behavior is like:
1. (network) Receives AcceptCHFrame
2. (network) Some required hints are missing and merge headers for restart.
3. (network -> browser) IPC to AcceptCHFrameReceived here.
4. (browser) restarts with fields required by ACCEPT CH frame
5. (network) AcceptCHFrame received.
6. (network) since all required headers exist, continue in the network layer. i.e. AcceptCHFrameReceived won't be called.Then, Line 1897-1903 might not be called after restart.
What do you think?
Hiroshige HayashizakiAgree that this method might not be called after restart if all the client hints are provided. Since the browser destructs the URL loader and creats a new one, it will be quite complicated to pass the restart time to the URL loader and record the latency to receive accept-CH frame again. Is measuring the time between the `NavigationURLLoaderImpl::Restart` and `NavigationURLLoaderImpl::OnReceiveResponse` or `NavigationURLLoaderImpl::OnReceiveRedirect` an acceptable rough estimate?
Er, perhaps the name "restart" was confusing -- `Restart()` is called for the initial request as well (https://source.chromium.org/chromium/chromium/src/+/main:content/browser/loader/navigation_url_loader_impl.cc;drc=8abea14deda089834ba142a35e8342014812df55;l=750).
So in the Yoshisato's scenario, there is Step 0:
0. (browser) start the initial navigation network request.
And this UMA measures the time between Step 0 and Step 4, i.e. a rough estimate of full restart turnaround time (IPCs cycle between browser process and network service) which would be ideally removed after we could move the restarting logic to the network service.
The latency until `OnReceiveResponse` is different and I expect it wouldn't work for this purpose, as it would contain the latency of the server and network connection etc.
Related details
- Jiacheng Guo
- Yoshisato Yanagisawa
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Yoshisato Yanagisawa (Gerrit)
Yoshisato Yanagisawa added 2 comments![]( "Open in Gerrit")
Still lgtm.
"Navigation.URLLoader.OnAcceptCHFrameReceived.RestartTime", restart_time);Jiacheng GuoWill this function be called?
I understand the behavior is like:
1. (network) Receives AcceptCHFrame
2. (network) Some required hints are missing and merge headers for restart.
3. (network -> browser) IPC to AcceptCHFrameReceived here.
4. (browser) restarts with fields required by ACCEPT CH frame
5. (network) AcceptCHFrame received.
6. (network) since all required headers exist, continue in the network layer. i.e. AcceptCHFrameReceived won't be called.Then, Line 1897-1903 might not be called after restart.
What do you think?
Hiroshige HayashizakiAgree that this method might not be called after restart if all the client hints are provided. Since the browser destructs the URL loader and creats a new one, it will be quite complicated to pass the restart time to the URL loader and record the latency to receive accept-CH frame again. Is measuring the time between the `NavigationURLLoaderImpl::Restart` and `NavigationURLLoaderImpl::OnReceiveResponse` or `NavigationURLLoaderImpl::OnReceiveRedirect` an acceptable rough estimate?
Er, perhaps the name "restart" was confusing -- `Restart()` is called for the initial request as well (https://source.chromium.org/chromium/chromium/src/+/main:content/browser/loader/navigation_url_loader_impl.cc;drc=8abea14deda089834ba142a35e8342014812df55;l=750).
So in the Yoshisato's scenario, there is Step 0:
0. (browser) start the initial navigation network request.
And this UMA measures the time between Step 0 and Step 4, i.e. a rough estimate of full restart turnaround time (IPCs cycle between browser process and network service) which would be ideally removed after we could move the restarting logic to the network service.
The latency until `OnReceiveResponse` is different and I expect it wouldn't work for this purpose, as it would contain the latency of the server and network connection etc.
Thanks for the explanation. That makes sense to me, then.
Related details
- Hiroshige Hayashizaki
- Jiacheng Guo
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |