Closed
Bug 142255
Opened 22 years ago
Closed 19 years ago
RFE provide a way to prioritize connections
Categories
(Core :: Networking, enhancement, P5)
Core
Networking
Tracking
()
RESOLVED
FIXED
mozilla1.8beta1
People
(Reporter: p_ch, Assigned: darin.moz)
References
(Blocks 1 open bug)
Details
Attachments
(1 file, 1 obsolete file)
26.72 KB,
patch
|
bzbarsky
:
review+
|
Details | Diff | Splinter Review |
Many part of the browser could benefit of this feature: - tabbed browsing: browsing on the current tab would not slow down - opening a bookmark group would display one page while the other page would continue to download in the background. - dl manager could have the ability to speedup one of its downloaded files.
Reporter | ||
Updated•22 years ago
|
Whiteboard: dupeme
Updated•22 years ago
|
more broader than file handling of downloads...
Component: Networking → File Handling
Keywords: helpwanted
Summary: Provide a way to prioritize downloads → Provide a way to prioritize connections
Whiteboard: dupeme
Assignee | ||
Comment 3•22 years ago
|
||
-> me
Assignee: new-network-bugs → darin
Component: File Handling → Networking
Summary: Provide a way to prioritize connections → RFE provide a way to prioritize connections
Assignee | ||
Comment 4•22 years ago
|
||
*** Bug 163730 has been marked as a duplicate of this bug. ***
Assignee | ||
Updated•22 years ago
|
Status: NEW → ASSIGNED
Priority: -- → P5
Target Milestone: --- → mozilla1.2beta
Assignee | ||
Comment 5•22 years ago
|
||
probably not going to happen for moz 1.2 -> future
Target Milestone: mozilla1.2beta → Future
Comment 6•20 years ago
|
||
Darin and I were just discussing this. We see the following priority levels (in order from most to least important): 1) Toplevel pages 2) Resources that block parsing of the page (scripts) 3) Resources that block onload but not parsing and will be shown (images) 4) Resources that block onload but not parsing and will not be shown (var foo = new Image()) 5) Resources that don't block onload (backgrounds) 6) Resources that don't affect any currently-requested pages (prefetch) We know loads of type #1 because the DOCUMENT_URI flag is set. For the rest, the idea is to introduce bits on nsIRequest (coopting the already-existing LOAD_BACKGROUND flag) to identify the load types. If we make it so that of the five non-DOCUMENT_URI types the last two have LOAD_BACKGROUND set and the others don't, we aren't even changing the API any. LOAD_NORMAL should correspond to level 3 in that list. Then we can flag script loads, image loads, etc as needed. Finally, we implement the prioritization itself in nsHttpConnectionMgr::ProcessNewTransaction. Basically, replace the line: 685 ent->mPendingQ.AppendElement(trans); With insertion into the list based on priority (the list should be sorted in priority order, with most important things near the front). biesi, how bored are you? ;)
Comment 7•20 years ago
|
||
(In reply to comment #6) > biesi, how bored are you? ;) not bored enough currently ;) Isn't that list missing downloads, and things like xmlhttprequest and webservices?
Comment 8•20 years ago
|
||
I think it makes sense to keep all those as LOAD_NORMAL...
Comment 9•20 years ago
|
||
I said it somewhere else, but adding the "Download Manager Tweak" would fix this and a WHOLE LOT of other DM issues.
->me
Status: NEW → ASSIGNED
Keywords: helpwanted
Assignee | ||
Comment 12•20 years ago
|
||
This patch implements support for scheduling of transactions based on an integral priority value set via nsIHttpChannelInternal::SetPriority. The default value is 0 with smaller values having higher priority (negative values are allowed). Priority values can be changed on a HTTP channel at any time, so this allows applications to schedule and reschedule requests as they see fit.
Assignee | ||
Updated•20 years ago
|
Assignee: cst → darin
Attachment #165815 -
Attachment is obsolete: true
Attachment #171102 -
Flags: review?(bzbarsky)
Assignee | ||
Updated•20 years ago
|
Target Milestone: Future → mozilla1.8beta
Comment 13•20 years ago
|
||
nsIHttpChannelInternal needs a new IID... but, should this maybe be on a more generic interface implementable by other channels?
Assignee | ||
Comment 14•20 years ago
|
||
whoops.. you're right. i forgot to rev the iid! as for using a more generic interface. yes! i absolutely want to do that, but my plan for that is to make use of the theorized new properties interface (bug 270224) that all channels would implement. nsIHttpChannelInternal would mostly go away when we have that interface, so i figured dumping one more attribute on nsIHttpChannelInternal was a reasonable intermediate measure. there are many issues with the properties interface, and i don't want to delay this patch waiting for that one.
Comment 15•19 years ago
|
||
Comment on attachment 171102 [details] [diff] [review] v1 patch - backend only r=bzbarsky. Unless the queue ever gets long, in which case it may make more sense to do a binary search... Alternately, we could do the search from the end, if we anticipate having lots of things all at the 0 priority (since in that case searching from the end is faster).
Attachment #171102 -
Flags: review?(bzbarsky) → review+
Assignee | ||
Comment 16•19 years ago
|
||
Yeah, I considered implementing a binary search, but since this algo is only run typically once per request, I don't expect its runtime to be that much of a factor.
Assignee | ||
Comment 17•19 years ago
|
||
bz: actually, i took your advice and rewrote InsertTransactionSorted like so: for (PRInt32 i=pendingQ.Count()-1; i>=0; --i) { nsHttpTransaction *t = (nsHttpTransaction *) pendingQ[i]; if (trans->Priority() >= t->Priority()) { pendingQ.InsertElementAt(trans, i+1); return; } } pendingQ.InsertElementAt(trans, 0); as a result, without any assigned priorities, this code ends up having constant runtime :)
Comment 18•19 years ago
|
||
Yep. Sounds good.
Assignee | ||
Comment 19•19 years ago
|
||
fixed-on-trunk now, on the consumers ;-)
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Comment 20•19 years ago
|
||
making use of this feature is done in bug 172362 / bug 47661, right?
Keywords: helpwanted
Whiteboard: active
Assignee | ||
Comment 21•19 years ago
|
||
yup. btw, i realized that nsHttpChannel::SetPriority should call nsHttpHandler::ResheduleTransaction, but it does not. that needs to be fixed. also, i've been thinking about how front-end code can toggle these priority values, and i think it might make sense to have a priority field live on each loadgroup that can be inherited by requests that are added to the loadgroup (this is nothing knew... bz's loadflag idea would have made this trivial to impl). i think it makes sense for there to be a common interface implemented by loadgroup and any requests that support prioritization. that means either the properties interface i've been wanting or a custom nsIPriorityRequest interface. that interface would need to be implemented by the image requests as well as the HTTP channels.
Comment 22•19 years ago
|
||
> and i think it might make sense to have a priority field live on each
> loadgroup that can be inherited by requests that are added to the loadgroup
hm... you'd generally want non-page requests (images, plugins, etc) have less
priority... Maybe the loadgroup version is just an "offset" to the priority of
the stuff added to the loadgroup, i.e. loadgroup::addrequest would do something
like:
aRequest.priority += mPriority;
instead of =.
Assignee | ||
Comment 23•19 years ago
|
||
> the stuff added to the loadgroup, i.e. loadgroup::addrequest would do something
> like:
> aRequest.priority += mPriority;
> instead of =.
biesi: that's exactly what i had in mind! we might also want to make
removeRequest decrement the priority of the request in case we are moving
requests from one loadgroup to another. i should probably file a bug for this.
Assignee | ||
Comment 24•19 years ago
|
||
I filed bug 278531 for the loadgroup stuff.
Comment 25•19 years ago
|
||
How about some guidance in the IDL as to what values to use for the priority? For example, kHighPriority = -100000 kHighPriority = -1000 kNormalPriority = 0 kLowPriority = 1000; kLowestPriority = 10000 Otherwise I have no idea what a good value is.
You need to log in
before you can comment on or make changes to this bug.
Description
•