1354409 - Add new telemetry probe for stats about racing results distribution

Status: RESOLVED FIXED

(Core :: Networking: Cache, enhancement)

Description

[Junior [inactive]]

To evaluate rcwn algorithm, we need new probe for a chart including
- how many times network was faster when we raced immediately
- how many times network was faster when we raced with a delay
- how many times network was slower when we raced immediately
- how many times network was slower when we raced with a delay

Comment 1

[Junior [inactive]]

Valentin has worked on it.

Comment 3

[Valentin Gosu [:valentin] (he/him)]

Attachment: Bug 1354409 - Add telemetry for how often we race network and cache

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

Comment 4

[Valentin Gosu [:valentin] (he/him)]

Attachment: Bug 1354409 - Make sure nsHttpChannel::mRacingNetAndCache is only true when we are actually racing

mRacingNetAndCache used to be set in TriggerNetwork, when the network was triggered before the cache callbacks had been called, but this could also happen when we bypassed the cache.

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

Comment 5

[Michal Novotny [:michal]]

Comment on attachment 8859415 [details]
Bug 1354409 - Make sure nsHttpChannel::mRacingNetAndCache is only true when we are actually racing

https://reviewboard.mozilla.org/r/131444/#review134124

::: netwerk/protocol/http/nsHttpChannel.cpp:8854
(Diff revision 1)
>      LOG(("nsHttpChannel::MaybeRaceNetworkWithCache [this=%p]\n", this));
> +
> +    if (!mOnCacheAvailableCalled) {
> +        // If the network was triggered before onCacheEntryAvailable was
> +        // called, we are either racing network and cache, or the load is
> +        // bypassing the cache.

This comment as well as the comment in header file need to be changed.

Comment 6

[Valentin Gosu [:valentin] (he/him)]

Comment on attachment 8859414 [details]
Bug 1354409 - Add telemetry for how often we race network and cache

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

Comment 7

[Valentin Gosu [:valentin] (he/him)]

Comment on attachment 8859415 [details]
Bug 1354409 - Make sure nsHttpChannel::mRacingNetAndCache is only true when we are actually racing

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

Comment 8

[Michal Novotny [:michal]]

Comment on attachment 8859415 [details]
Bug 1354409 - Make sure nsHttpChannel::mRacingNetAndCache is only true when we are actually racing

https://reviewboard.mozilla.org/r/131444/#review136904

Comment 9

[Michal Novotny [:michal]]

Comment on attachment 8859414 [details]
Bug 1354409 - Add telemetry for how often we race network and cache

https://reviewboard.mozilla.org/r/131442/#review138040

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f15be762532c
Add telemetry for how often we race network and cache r=michal
https://hg.mozilla.org/integration/autoland/rev/d08698232edf
Make sure nsHttpChannel::mRacingNetAndCache is only true when we are actually racing r=michal

Comment 11

[Michal Novotny [:michal]]

This will report kDidNotRaceUsedNetwork and kDidNotRaceUsedCache even if racing is disabled, but we should report the values only when racing is enabled. Could you please fix it here or file a follow up bug?

Comment 12

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

https://hg.mozilla.org/mozilla-central/rev/f15be762532c
https://hg.mozilla.org/mozilla-central/rev/d08698232edf

Comment 13

[Junior [inactive]]

(In reply to Michal Novotny (:michal) from comment #11)
> This will report kDidNotRaceUsedNetwork and kDidNotRaceUsedCache even if
> racing is disabled, but we should report the values only when racing is
> enabled. Could you please fix it here or file a follow up bug?

I think I can fix it in bug 1354405 together.

Comment 14

[Valentin Gosu [:valentin] (he/him)]

(In reply to Junior[:junior][ooo 5/11-16] from comment #13)
> > enabled. Could you please fix it here or file a follow up bug?
> I think I can fix it in bug 1354405 together.
Thanks, Junior!