Closed Bug 1362071 Opened 7 years ago Closed 6 years ago

[meta] Improve h1 connection/h2 stream download throttling

Categories

(Core :: Networking: HTTP, enhancement, P2)

enhancement

Tracking

()

RESOLVED FIXED

People

(Reporter: mayhemer, Assigned: mayhemer)

References

Details

(Whiteboard: [necko-triaged])

Details to come.
Blocks: 1360793
Assignee: nobody → honzab.moz
Whiteboard: [necko-active]
Priority: -- → P1
Implementation details:

- throttle solely by stopping read from the socket: this will be implemented in nsHttpTransaction, simply when a transaction is about to stop reading, it returns NS_ERROR_WOULD_BLOCK from nsHttpTransaction::WriteSegments.  this makes either nsHttpConnection or Http2Stream wait for call to ResumeRecv on itself ; hence no need for bug 1355782, this will protocol-independent 

- remove the code that suspends and resumes channels on pressure periodically since it doesn't make much sense to keep it
- we also don't need to keep the pressure indication at all, enough is to track number of active transactions per-window

- keep active transactions (on the wire) in two hashtables by window id, one for non-throttled (mUnthrottledTransx) and one for throttled transactions (mThrottledTransx) ; "throttled" means having the Throttleable class flag ; must keep in mind the flag can be set at any time during the transaction lifetime, hence active transactions must "travel" between the two hashtables (all on socket thread only)

- an active transaction T should be throttled when tab A is active as (ShouldThrottle(T), checked in each call of nsHttpTransaction::WriteSegments, hence, must be optimized):

  if (mUnthrottledTransx[A].count() || mThrottledTransx[A].count()) {
    // Something happens for the active tab
    if (!T.ForTab(A)) 
      return true; // this is a background tab load
    if (mUnthrottledTransx[A].count() > 0) 
      return T.class & Throttleable; // there are unthrottled tranx in progress, respect our throttling
    return false;
  }
  // the active tab is not loading anything

  if (mUnthrottledTransx[*].count() > 0)
    return T.class & Throttleable; // there are in progress unthrottleable tranx, respect own throttling
  return false; // there are only throttleable transactions left in progress


- there should be a single timer (as we have now), that periodically ticks
  - the ticker (3s/200ms) will set/unset a global "stop reading" flag
  - on unset of the flag by the timer
    -> iterate all m(Un)ThrottledTransx arrays to wake them up (except the mUntrottledTransx[A], obviously)

- on add to any of m(Un)TrottledTransx and timer being off
  -> set the "stop reading" global flag and start the timer

- when "stop reading" is ON:
  - on change to the active tab (A is the new active tab id)
    -> iterate all mUnthrottledTransx[A] arrays to wake them up, if non-empty, and exit
    -> iterate all mThrottledTransx[A] arrays to wake them up, if non-empty, and exit
    -> iterate all remaning mUnthrottledTransx arrays to wake them up, exit if some woken
    -> iterate all remaning mThrottledTransx arrays to wake them up, exit if some woken
  - on emptying of mUnthrottledTransx[A]
    -> iterate all mThrottledTransx[A] arrays to wake them up, if non-empty, and exit
    -> iterate all remaning mUnthrottledTransx arrays to wake them up, exit if some woken
    -> iterate all remaning mThrottledTransx arrays to wake them up, exit if some woken
  - on emptying of mThrottledTransx[A] while also mUnthrottledTransx[A] being empty
    -> iterate all remaning mUnthrottledTransx arrays to wake them up, exit if some woken
    -> iterate all remaning mThrottledTransx arrays to wake them up, exit if some woken
  - similarly for complete of non-active...
  - and finally, when nothing active 
    -> stop the timer and make sure "stop reading" is false

- note that there is no need to 'put transx to sleep' since all will automatically stop reading based on result of ShouldThrottle that will automatically drop to false when conditions change


This should, if present, throttle everything but:
1. non-throttled for the active tab
2. throttled for the active tab
3. non-throttled for background tabs
4. throttled for background tabs

Nice is that we can play with the "0"s in ShouldThrottle function.  We could allow some threshold when to open floodgates.

I will split this to separate bugs.
Depends on: 1365306
No longer depends on: 1357614
No longer depends on: 1360583
Depends on: 1360580
No longer blocks: 1360793
Depends on: 1365307
No longer depends on: 1355782
Depends on: 1360603
Depends on: 1366822
Depends on: 1369496
Depends on: 1367861
This is an assigned P1 bug without activity in two weeks. 

If you intend to continue working on this bug for the current release/iteration/sprint, remove the 'stale-bug' keyword.

Otherwise we'll reset the priority of the bug back to '--' on Monday, August 28th.
Keywords: stale-bug
Priority: P1 → P2
Keywords: stale-bug
Whiteboard: [necko-active]
Whiteboard: [necko-triaged]
I don't think we need to track the remaining issue.
Status: NEW → RESOLVED
Closed: 6 years ago
No longer depends on: 1367861
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.