1425301 - Progressively shorten the 6 seconds max tailing delay with time since nav start (may cause indefinite tailing)

Status: RESOLVED FIXED

(Core :: Networking, defect, P2)

Description

[Honza Bambas (:mayhemer)]

Discovered when investigating bug 1423373.  The tailing delay of tracker requests is restarted on every non-tailed request opening or finishing.  If there is a repetitive non-tailed request we may never untail.

the "quantum" I'm calculating the delay with may be divided by (or subtracted) the number of seconds (or some (semi)monotonic function of it) from the load start time.  This would keep tailed requests tailed when there is a lot of activity, but also would make sure we untail at a certain moment.

Comment 1

[Honza Bambas (:mayhemer)]

P2 since in combo with 1425296 it may cause an indefinite page load

Comment 2

[Honza Bambas (:mayhemer)]

Attachment: wip1

backup, I'm also thinking about dropping to zero when the calculated maximum drops below certain threshold of say like 200ms.

Comment 3

[Dragana Damjanovic [:dragana]]

Comment on attachment 8938399 [details] [diff] [review]
wip1

Review of attachment 8938399 [details] [diff] [review]:
-----------------------------------------------------------------

looks good. one question:

do we want a real hard deadline? like a 1min?

::: netwerk/base/RequestContextService.cpp
@@ +261,5 @@
>  
> +  if (!mBeginLoadTime.IsNull()) {
> +    // This seems to me like a reasonable heuristic.  When the loading time
> +    // is twice the max delay (12s), we shorten the tail timer delay to only
> +    // 1 second on every new or finished regular request.  In 24 seconds

the explanation is a bit confusing. you mean 24 after the first 24 second...
please rephrase.

Comment 4

[Honza Bambas (:mayhemer)]

(In reply to Dragana Damjanovic [:dragana] from comment #3)
> Comment on attachment 8938399 [details] [diff] [review]
> wip1
> 
> Review of attachment 8938399 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> looks good. one question:
> 
> do we want a real hard deadline? like a 1min?

Probably even shorter...  45 seconds maybe?  I think 45 secs is already quite a degraded performance, so we probably can start loading trackers after that time.  Note that this is a totally arbitrarily chosen number.

Let's have "network.http.tailing.total-delay-max" then?

> 
> ::: netwerk/base/RequestContextService.cpp
> @@ +261,5 @@
> >  
> > +  if (!mBeginLoadTime.IsNull()) {
> > +    // This seems to me like a reasonable heuristic.  When the loading time
> > +    // is twice the max delay (12s), we shorten the tail timer delay to only
> > +    // 1 second on every new or finished regular request.  In 24 seconds
> 
> the explanation is a bit confusing. you mean 24 after the first 24 second...
> please rephrase.

I am about to change the calculation so the comment will change as well.

Thanks!

Comment 5

[Honza Bambas (:mayhemer)]

Attachment: v1

So, I simply linearly go to 0 with the maxDelay value between load begin and 45 seconds (configurable).  Hence, in ~23 seconds after load start the max delay will be 3 seconds, after 45 seconds and on will be 0, means to untail immediately.  I think this is a clear and well predictable heuristic.

Comment 6

[Dragana Damjanovic [:dragana]]

Comment on attachment 8939898 [details] [diff] [review]
v1

Review of attachment 8939898 [details] [diff] [review]:
-----------------------------------------------------------------

Yes, this is a better heuristic. Thanks.

::: netwerk/base/RequestContextService.cpp
@@ +267,5 @@
> +
> +    uint32_t sinceBeginLoad = static_cast<uint32_t>(
> +      (TimeStamp::NowLoRes() - mBeginLoadTime).ToMilliseconds());
> +    uint32_t tillTotal = totalMax - std::min(sinceBeginLoad, totalMax);
> +    uint32_t proportion = (delayMax * tillTotal) / totalMax; // values clamped to 60'000

totalMax can be set to zero, please check before the calculation.

Comment 7

[Honza Bambas (:mayhemer)]

Attachment: v1.1

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/cf08f8b8d13b
Progressively decrease the 6 seconds maximum quantum delay of tracker script tailing, r=dragana

Comment 9

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/cf08f8b8d13b

Comment 10

[Gerry Chang [:gchang]]

It's too late for 58. Mark 58 won't fix.