Closed
Bug 1405446
Opened 7 years ago
Closed 7 years ago
h1 urgent start connections should be used only for urgent start requests
Categories
(Core :: Networking: HTTP, enhancement, P2)
Core
Networking: HTTP
Tracking
()
RESOLVED
FIXED
mozilla59
People
(Reporter: mayhemer, Assigned: mayhemer)
References
Details
(Whiteboard: [necko-triaged])
Attachments
(1 file, 2 obsolete files)
27.09 KB,
patch
|
dragana
:
review+
|
Details | Diff | Splinter Review |
since otherwise we may soon exhaust/block them when multiple pages to the same hosts are open or when a single user interaction leads to more then 4 requests. this means marking a connection as urgent-start (flag) and probably also effectively renders bug 1345845 invalid, since urgent start conns will, when not utilized, close soon by our timeout.
![]() |
Assignee | |
Updated•7 years ago
|
Assignee: nobody → honzab.moz
Status: NEW → ASSIGNED
Priority: -- → P2
![]() |
Assignee | |
Comment 1•7 years ago
|
||
Updated•7 years ago
|
Whiteboard: [necko-triaged]
![]() |
Assignee | |
Comment 2•7 years ago
|
||
still has issues. I can clearly see the improvement when loading multiple pages from the same domain - we immediately load the html in every case - but there is another issue: parallelism sometimes go to hell for the active page. I'm not sure right now about the cause. I have to investigate a bit more time to this, but according importance, I don't think I want right now.
Attachment #8917474 -
Attachment is obsolete: true
![]() |
Assignee | |
Comment 3•7 years ago
|
||
Comment on attachment 8918915 [details] [diff] [review] wip2 Review of attachment 8918915 [details] [diff] [review]: ----------------------------------------------------------------- ::: netwerk/protocol/http/nsHttpConnectionMgr.cpp @@ +1609,5 @@ > + // to dispatch on an urgent-start-only marked connection to avoid > + // dispatch deadlocks > + if (!(trans->ClassOfService() & nsIClassOfService::UrgentStart) && > + idleConnsAllUrgent && > + !ent->mActiveConns.Length()) { ent->mActiveConns.Length() < 6 will probably do. I think this is the point ruining parallelism. When there are only urgent idle conns, we use one of them here (becomes active). for another trans in the row this cond will no pass (!ent->mActiveConns.Length() evals to false)
![]() |
Assignee | |
Comment 4•7 years ago
|
||
Locally tested, observed very good performance when loading multiple pages from the same domain (e.g bbc.com). Parallelism doesn't suffer. how urgent dispatching works w/o this patch: - we have a regular limit of 6 conns per host - we can create up to 4 more conns for urgent transactions (total of 10 conns) - all these are then used for both regular and urgent transactions to dispatch, allowing to make the limit higher and when all used, disallow usage of the urgent-start feature at all.. the goal of the patch is to limit number of active connections carrying only regular transactions back to 6 while allowing connections that are urgent-preferred free to reuse by leaving them idle ; these urgent-preferred conns will be used for any urgent transactions immediately when it happens that there is e.g 8 urgent conns and 2 regular, we allow usage of urgent-preferred conns for regular transactions when active conns count (both regular and urgent summed) is less than 6 this ensures that parallelism is 6 while we save the reserve for urgency when it comes in the patch: - marks an nsHttpConnection resulting from urgent-start initiated speculative connection as "urgent preferred" - marks an nsHttpConnection resulting from a regular speculative connection as "urgent preferred" when the first non-null transaction dispatched on it is urgent - when trying to dispatch a transaction (from the queue or a new high-prio one) which is regular, we first look for a regular idle conn ; if none is found, but we can't make new connections because of the 6 per-host limit although active connections are less than 6 (so that we can dispatch), we force use of an urgent-preferred conn for a regular transaction (the "repeat step 2" part)
Attachment #8918915 -
Attachment is obsolete: true
Attachment #8941096 -
Flags: review?(dd.mozilla)
Comment hidden (obsolete) |
![]() |
Assignee | |
Comment 6•7 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=fc9b9ab84fd0aa5fbfb089c1b3da0d4b2c9446c7
Updated•7 years ago
|
Attachment #8941096 -
Flags: review?(dd.mozilla) → review+
Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/f6bd79484f23 Connections created for urgent-start requests are of limits for non-urgent-start ones, r=dragana
Keywords: checkin-needed
Comment 9•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/f6bd79484f23
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox59:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
Updated•7 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•