Closed Bug 142255 Opened 21 years ago Closed 18 years ago

RFE provide a way to prioritize connections

Categories

(Core :: Networking, enhancement, P5)

enhancement

Tracking

()

RESOLVED FIXED
mozilla1.8beta1

People

(Reporter: p_ch, Assigned: darin.moz)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

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.
Whiteboard: dupeme
Blocks: prefetch
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
I mean "broader" sorry about the bad English.
-> me
Assignee: new-network-bugs → darin
Component: File Handling → Networking
Summary: Provide a way to prioritize connections → RFE provide a way to prioritize connections
*** Bug 163730 has been marked as a duplicate of this bug. ***
Status: NEW → ASSIGNED
Priority: -- → P5
Target Milestone: --- → mozilla1.2beta
probably not going to happen for moz 1.2 -> future
Target Milestone: mozilla1.2beta → Future
Blocks: 190585
Blocks: 245093
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?  ;)
(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?
I think it makes sense to keep all those as LOAD_NORMAL...
I said it somewhere else, but adding the "Download Manager Tweak" would fix this
and a WHOLE LOT of other DM issues.
->me
Assignee: darin → cst
Status: ASSIGNED → NEW
Keywords: helpwanted
Whiteboard: active
Blocks: 277469
No longer blocks: 277469
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: cst → darin
Attachment #165815 - Attachment is obsolete: true
Attachment #171102 - Flags: review?(bzbarsky)
Target Milestone: Future → mozilla1.8beta
nsIHttpChannelInternal needs a new IID... but, should this maybe be on a more
generic interface implementable by other channels?
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.
Blocks: 277469
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+
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.
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 :)
Yep.  Sounds good.
fixed-on-trunk

now, on the consumers ;-)
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Blocks: 172362
making use of this feature is done in bug 172362 / bug 47661, right?
Keywords: helpwanted
Whiteboard: active
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.
> 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 =.
> 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.
I filed bug 278531 for the loadgroup stuff.
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.