Closed Bug 1348819 Opened 3 years ago Closed 3 years ago

Potentially limit total connections for hosts referred by background tabs.

Categories

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

enhancement

Tracking

()

RESOLVED WONTFIX
Tracking Status
firefox55 --- affected

People

(Reporter: mayhemer, Assigned: kershaw)

References

(Blocks 1 open bug)

Details

(Whiteboard: [necko-active])

Attachments

(1 file)

To give as much as possible bandwidth to the active tab, we may want to limit total active/half-open connections to hosts coming from references from background tabs.  This should further improve prioritization of the active tab's bandwidth, beyond bug 1312782.
Whiteboard: [necko-active]
Assignee: nobody → honzab.moz
(If there is anyone willing to take this, please do.  I only want to remove this from the necko triage list.)
(In reply to Honza Bambas (:mayhemer) from comment #1)
> (If there is anyone willing to take this, please do.  I only want to remove
> this from the necko triage list.)

I'd like to take this.
Assignee: honzab.moz → kechang
Attached patch WIPSplinter Review
Summary:
 - Check if all pending transactions are from non-focused window when:
   1. Adding new transaction
   2. Top level window id is changed.
 - If all pending transactions in an entry are from non-focused window, limit total connections

Please note this is just a quick patch for making sure I understand your goal correctly. Thanks.
Attachment #8854844 - Flags: feedback?(honzab.moz)
I don't think the patch is what we want, but I will think about this more deeply before I can give you a feedback.

Note that this is not a high priority bug but only a "nice to have" and it's OK it misses the deadline, so please don't waste much time on it.

Thanks.
Priority: -- → P3
Comment on attachment 8854844 [details] [diff] [review]
WIP

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

Not sure it's what we exactly want here.  The goal is an overall (cross origin) limit for inactive tab active conns.

Currently at [1] we iterate as |for (mCT) Dispatch(mCT[i]->trans[])|, regardless the transactions are for active or inactive tabs and regardless how prioritized they are in the whole, cross origin view.  Maybe the first step would be to change that to iterate |for (mCT) DispatchIfForActive(mCT[i]->trans[]); for (mCT) DispatchIfForInactive(mCT[i]->trans[]);| - very schematically ;)

In ideal world, this should be |DispatchIfActive(allTransByPriority[], overallLimit = ~100); DispatchIfInactive(allTransByPriority[], overallLimit = ~10);|  Do we have a complete view/array of all pending transactions?  I think we don't.

Other change could be to modify AtActiveConnectionLimit to use different pref for active tab trans and for inactive tab trans.  Like:

AtActiveConnectionLimit(nsConnectionEntry *ent, nsHttpTransaction *trans)
{
  ..
  maxSocketCount = trans->winID == mActiveWinID ? gHttpHandler->MaxSocketCount() /* 100 */ : gHttpHandler->MaxInactiveSocketCount() /* 10 */;
  ..
}

Although, to make this work correctly, we need to sum active transactions individually for the active and inactive tabs.  I'm more and more thinking of having an additional table in conn-manager that would collect all pending transactions mapped as [winid] -> [transaction array by priority].  When looking for something to initially dispatch at [1], that table would be consulted rather than each conn entry individually.

I think we should discuss this in person as the whole team.


[1] https://dxr.mozilla.org/mozilla-central/rev/b043233ec04f06768d59dcdfb9e928142280f3cc/netwerk/protocol/http/nsHttpConnectionMgr.cpp#2330

::: netwerk/protocol/http/nsHttpConnectionMgr.cpp
@@ +1027,5 @@
>      }
>  
> +    if (ent->mFromNonFocusedWindow) {
> +        // XXX Hard coded to 3 currently.
> +        // What if the entry is using proxy? The max connection should be?

The same?
Attachment #8854844 - Flags: feedback?(honzab.moz) → feedback-
We found a different solution(s) because this is overly complicated in ratio to the benefit.
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.