Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify stream priority. #453

Merged
merged 1 commit into from
Apr 23, 2014
Merged

Conversation

jpinner
Copy link

@jpinner jpinner commented Apr 12, 2014

The priority groups are abandoned in favor of a single weighted dependency tree.
The root node of the tree is the connection, assigned the stream identifier 0x0.
By default, streams are assigned a stream dependency of 0x0 with a weight of 16.

@tatsuhiro-t
Copy link
Contributor

Just read through it, I think it looks good.

I'd like to understand how weight of the stream is calculated and changed. Here is my understanding:
The effective weight of each stream is determined by traversing tree node, taking into account of distribution of weight of parent stream among siblings. This effective weight is temporal on the tree and initial weight is unchanged for a stream, so when reprioritizing that stream, its weight is initial one. For example, suppose we have the following dependency tree:

0x0 <--(16)-- A <--(4)-- B <--(16)-- D
                 \-(8)-- C

X<--(w)--Y means that Y depends on X with weight w (and w is initial weight).
Calculated effective weight is like this:

A: 16
B: 16*(1/3)
C: 16*(2/3) 
D: 16*(1/3)

If parent stream is closed, then its initial weight is distributed to among the child streams and this updates initial weight of child stream.
So, after A was closed, the initial weight of streams becomes like this:

0x0 <--(16*(1/3))-- B <--(16)-- D
     \-(16*(2/3))-- C

@martinthomson
Copy link
Collaborator

@jpinner thanks for this. I'll have to read through this more carefully before I comment.

@tatsuhiro-t that seems right. Note that this means that a server might want to maintain more precision on it's weight values than the 8 bits we mandate in the protocol. That said, if there are deep dependency trees and you are killing off nodes that are high up, at some point you will need to round off, so I think that how much precision is allocated is largely a discretionary thing.

@grmocg
Copy link
Contributor

grmocg commented Apr 12, 2014

Looks good.

On Sat, Apr 12, 2014 at 11:33 AM, Martin Thomson
notifications@github.comwrote:

@jpinner https://github.com/jpinner thanks for this. I'll have to read
through this more carefully before I comment.

@tatsuhiro-t https://github.com/tatsuhiro-t that seems right. Note that
this means that a server might want to maintain more precision on it's
weight values than the 8 bits we mandate in the protocol. That said, if
there are deep dependency trees and you are killing off nodes that are high
up, at some point you will need to round off, so I think that how much
precision is allocated is largely a discretionary thing.

Reply to this email directly or view it on GitHubhttps://github.com//pull/453#issuecomment-40288069
.

@tatsuhiro-t
Copy link
Contributor

It seems that we can make a stream to depend on stream 0x0 exclusively, which makes all existing connections its descendant. This is not possible in the current draft version. I wonder this can be directly proxy-able since stream 0x0 is imaginable stream.

@grmocg
Copy link
Contributor

grmocg commented Apr 15, 2014

Yea, the idea is that '0' is a reserved stream ID, and so it is safe to use
it as a flag for saying that (effectively) any stream with '0' as a parent
has no parent.

On Mon, Apr 14, 2014 at 7:57 PM, Tatsuhiro Tsujikawa <
notifications@github.com> wrote:

It seems that we can make a stream to depend on stream 0x0 exclusively,
which makes all existing connections its descendant. This is not possible
in the current draft version. I wonder this can be directly proxy-able
since stream 0x0 is imaginable stream.


Reply to this email directly or view it on GitHubhttps://github.com//pull/453#issuecomment-40441004
.

@tatsuhiro-t
Copy link
Contributor

Well, then we should explicitly say that receiver must treat exclusive bit as 0 if stream ID field is 0.

@hruellan
Copy link
Contributor

@martinthomson I would add an explicit hint to implementers that having more than 8 bits of internal precision is a good thing.

@tatsuhiro-t I think we should leave open the possibility to insert a new stream between the connexion and all its children, even if it's probably a large re-prioritization.

@tatsuhiro-t
Copy link
Contributor

I think large re-prioritization is not a problem. If proxy gets such requests and it aggregates several frontend connections to single backend connection, inserting under stream 0x0 with exclusive flag set seems to be impossible in backend.

@jpinner
Copy link
Author

jpinner commented Apr 15, 2014

If it aggregates to a different backend connection it is not impossible to proxy.

Consider streams A and B depend on stream 0x0 and are proxied to some backend connection as streams X and Y. If we get a new streams, C, that exclusively depends on 0x0, we can proxy it as stream Z by dropping the exclusive dependency and then re-prioritizing streams X and Y to depend on stream Z.

@tatsuhiro-t
Copy link
Contributor

Yes, it works like that.

But if we just don't allow 0x0 with exclusive dependency, we can do all priority stuff via proxy without doing any extra handling except for stream ID mappings.

@jpinner
Copy link
Author

jpinner commented Apr 15, 2014

Yep, true -- I don't have much of a preference -- if clients feel like it is not an important use case than I am perfectly happy to disallow it.

@willchan
Copy link
Contributor

Is it possible to move this discussion to the mailing list? If you need
comments from a client, I'm happy to take a look later.

On Tue Apr 15 2014 at 9:39:56 AM, Jeff Pinner notifications@github.com
wrote:

Yep, true -- I don't have much of a preference -- if clients feel like it
is not an important use case than I am perfectly happy to disallow it.


Reply to this email directly or view it on GitHubhttps://github.com//pull/453#issuecomment-40504411
.

@mnot mnot added the proposal label Apr 18, 2014
@martinthomson martinthomson merged commit 66d3ded into httpwg:master Apr 23, 2014
@jpinner jpinner deleted the priority_update branch July 8, 2014 17:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants