The default bug view has changed. See this FAQ.

Sort Idle HTTP Connections by CWND

RESOLVED FIXED

Status

()

Core
Networking: HTTP
--
enhancement
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: mcmanus, Assigned: mcmanus)

Tracking

({dev-doc-complete})

Trunk
dev-doc-complete
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

6 years ago
Right now the idle persistent connection pool is a FIFO.

What really distinguishes different connections to the same server is the size of the sending congestion window (CWND) on the server. If the window is large enough to support the next response document then it can all be transferred (by definition) in 1 RTT. 

Connections with smaller windows are going to be limited by the RTT while they grow their windows.

All else being equal, which as far as I can tell it is, we want to use the big ones. We cannot directly tell what the server's CWND is of course, but the history of the connection provides a clue - connections which have moved large flights of data (single responses, or aggregate pipelines of responses) will have given the server the best chance for opening that window in the past.

we should sort the pool according to that metric.

I've done an experiment to show the best case - a link to a 25KB resource off of a page that contains a mixture of small and large content. In both cases the 25KB resource is loaded with an idle persistent connection. In the historic case it reuses a connection that had loaded a small image previously and it takes 3RTT (793ms) to transfer it.. in the case of sorting by cwnd the window is large enough to accommodate the entire resource and it is all complete in 1 RTT (363ms). Cool!

A much longer description of this along with pretty graphs is here: http://bitsup.blogspot.com/2011/01/firefox-idle-connection-selection-via.html

I don't really see a downside - the current algorithm is more or less a random selection and the worst case of sort-by-cwnd devolves into that. The code is also very simple - a lot simpler than this explanation.
(Assignee)

Updated

6 years ago
Assignee: nobody → mcmanus
Status: NEW → ASSIGNED
(Assignee)

Comment 1

6 years ago
Created attachment 502808 [details] [diff] [review]
sort idle pconn pool by cwnd v1

this will conflict with the patch in 604796 - but that will be easy to cleanup. I thought it better to submit without a dependency.
Attachment #502808 - Flags: review?(jduell.mcbugs)
Attachment #502808 - Flags: review?(jduell.mcbugs) → review?(honzab.moz)
(Assignee)

Comment 2

6 years ago
Created attachment 524549 [details] [diff] [review]
sort idle pconn pool by cwnd v2

update bitrot - candidate for firefox 5
Attachment #502808 - Attachment is obsolete: true
Attachment #524549 - Flags: review?
Attachment #502808 - Flags: review?(honzab.moz)
(Assignee)

Updated

6 years ago
Blocks: 604796
(Assignee)

Updated

6 years ago
Attachment #524549 - Flags: review? → review?(jduell.mcbugs)
Comment on attachment 524549 [details] [diff] [review]
sort idle pconn pool by cwnd v2

>+    PRInt64                         mBytesRead;      // data read per activation
>+    PRInt64                         mMaxBytesRead;   // max read in 1 activation

Change name of mBytesRead to mCurrentBytesRead to make it clear that its the bytes for the current activation, not "per activation."

>+
>+            PRInt32 idx;
>+            for (idx = 0; idx < ent->mIdleConns.Length(); idx++) {
>+                nsHttpConnection *idleConn = ent->mIdleConns[idx];
>+                if (idleConn->MaxBytesRead() < conn->MaxBytesRead())
>+                    break;
>+            }
>+

Add comment that this is inefficient way to sort list, but that's ok since it's only 6 elements long max.

+r with those fixes.
Attachment #524549 - Flags: review?(jduell.mcbugs) → review+
(Assignee)

Comment 4

6 years ago
Created attachment 524586 [details] [diff] [review]
sort idle pconn pool by cwnd v3

updates from comment 3.. r=jduell
Attachment #524549 - Attachment is obsolete: true
(Assignee)

Comment 5

6 years ago
pushed to cedar
http://hg.mozilla.org/projects/cedar/rev/198b15a65432
Comment on attachment 524586 [details] [diff] [review]
sort idle pconn pool by cwnd v3

carry forward review next time (and obsolete prev verison)
Attachment #524586 - Flags: review+
Pushed to m-c: http://hg.mozilla.org/mozilla-central/rev/198b15a65432
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Keywords: dev-doc-needed
What developer documentation changes need to be made? There appear to be no exposed API changes in this patch, and the only documentation we have that even mentions the HTTP transaction model is this:

https://developer.mozilla.org/en/HTTP_Transaction_Model

But this article is very old and is tagged as being badly out of date. If someone would like to tackle a revamp of that article (or provide me with notes that I can use to bring it up to date), that would be fabulous.
I have added a note to Firefox 5 for developers about this to cover things until/unless that other article gets updated:

https://developer.mozilla.org/en/Firefox_5_for_developers#HTTP
This is done; filed bug 650563 for updating the HTTP transaction model documentation.
Keywords: dev-doc-needed → dev-doc-complete
Depends on: 650871
You need to log in before you can comment on or make changes to this bug.