Skip to content

http2: calculate a correct window increment size for a stream #155

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 2 commits into from
Closed

http2: calculate a correct window increment size for a stream #155

wants to merge 2 commits into from

Conversation

ZekeLu
Copy link
Contributor

@ZekeLu ZekeLu commented Oct 22, 2022 •
edited
Loading

CL 432038 reduces sending WindowUpdates by introducing a threshold. Once
the remaining bytes are below the threshold, a single WindowUpdate is
sent to reset the amount back to the maximum amount configured.

The window increment size for a stream is calculated from:

sc.srv.initialStreamRecvWindowSize() - st.inflow.available()

Where (*flow).available is defined as:

func (f *flow) available() int32 {
	n := f.n
	if f.conn != nil && f.conn.n < n {
		n = f.conn.n
	}
	return n
}

When f.conn.c < f.n, it gets a bigger increment size. It should be
calculated from:

sc.srv.initialStreamRecvWindowSize() - st.inflow.n

While we're here, remove an unnecessary type conversion too.

Updates golang/go#56315.

@ZekeLu
http2: calcualte a correct window increment size for a stream
60a1247 
CL 432038 reduces sending WindowUpdates by introdcing a threshold. Once
the remaining bytes are below the threshold, a single WindowUpdate is
sent to reset the amount back to the maximum amount configured.

The window increment size for a stream is calculated from:

  sc.srv.initialStreamRecvWindowSize() - st.inflow.available()

Where (*flow).available is defined as:

  func (f *flow) available() int32 {
  	n := f.n
  	if f.conn != nil && f.conn.n < n {
  		n = f.conn.n
  	}
  	return n
  }

When f.conn.c < f.n, it gets a bigger increment size. It should be
calcualted from:

  sc.srv.initialStreamRecvWindowSize() - st.inflow.n

While at here, remove an unnecessary type conversion too.
@gopherbot
Copy link
Contributor

This PR (HEAD: 60a1247) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/net/+/444816 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

Message from Damien Neil:

Patch Set 1: Run-TryBot+1

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/444816.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gopher Robot:

Patch Set 1:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/444816.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gopher Robot:

Patch Set 1: TryBot-Result+1

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/444816.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

This PR (HEAD: 02fc09c) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/net/+/444816 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

Message from Zeke Lu:

Patch Set 2:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/444816.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Damien Neil:

Patch Set 2: Run-TryBot+1

Please don’t reply on this GitHub thread. Visit golang.org/cl/444816.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gopher Robot:

Patch Set 2:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/444816.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gopher Robot:

Patch Set 2: TryBot-Result+1

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/444816.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Damien Neil:

Patch Set 2:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/444816.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Damien Neil:

Patch Set 2: Code-Review+2

Please don’t reply on this GitHub thread. Visit golang.org/cl/444816.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Damien Neil:

Patch Set 2: Auto-Submit+1

Please don’t reply on this GitHub thread. Visit golang.org/cl/444816.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Ian Lance Taylor:

Patch Set 2:

(3 comments)

Please don’t reply on this GitHub thread. Visit golang.org/cl/444816.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Zeke Lu:

Patch Set 2:

(4 comments)

Please don’t reply on this GitHub thread. Visit golang.org/cl/444816.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Ian Lance Taylor:

Patch Set 3:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/444816.
After addressing review feedback, remember to publish your drafts!

@ZekeLu ZekeLu changed the title http2: calcualte a correct window increment size for a stream http2: calculate a correct window increment size for a stream Oct 27, 2022
@gopherbot
Copy link
Contributor

Message from Zeke Lu:

Patch Set 4:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/444816.
After addressing review feedback, remember to publish your drafts!

gopherbot pushed a commit that referenced this pull request Oct 27, 2022
@ZekeLu @gopherbot
http2: calculate a correct window increment size for a stream
0c1aede 
CL 432038 reduces sending WindowUpdates by introducing a threshold. Once
the remaining bytes are below the threshold, a single WindowUpdate is
sent to reset the amount back to the maximum amount configured.

The window increment size for a stream is calculated from:

    sc.srv.initialStreamRecvWindowSize() - st.inflow.available()

Where (*flow).available is defined as:

    func (f *flow) available() int32 {
    	n := f.n
    	if f.conn != nil && f.conn.n < n {
    		n = f.conn.n
    	}
    	return n
    }

When f.conn.c < f.n, it gets a bigger increment size. It should be
calculated from:

    sc.srv.initialStreamRecvWindowSize() - st.inflow.n

While we're here, remove an unnecessary type conversion too.

Updates golang/go#56315.

Change-Id: I4b26b27e4c5c5cd66e6a32b152d68f304adc65d8
GitHub-Last-Rev: 02fc09c
GitHub-Pull-Request: #155
Reviewed-on: https://go-review.googlesource.com/c/net/+/444816
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Run-TryBot: Damien Neil <dneil@google.com>
@gopherbot
Copy link
Contributor

Message from Ian Lance Taylor:

Patch Set 4: Run-TryBot+1 Auto-Submit+1 Code-Review+1

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/444816.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

This PR is being closed because golang.org/cl/444816 has been merged.

@gopherbot gopherbot closed this Oct 27, 2022
@ZekeLu ZekeLu deleted the increment-size branch October 27, 2022 05:07
WeiminShang added a commit to WeiminShang/net that referenced this pull request Nov 16, 2022
@WeiminShang @gopherbot
http2: calculate a correct window increment size for a stream
d9416fc 
CL 432038 reduces sending WindowUpdates by introducing a threshold. Once
the remaining bytes are below the threshold, a single WindowUpdate is
sent to reset the amount back to the maximum amount configured.

The window increment size for a stream is calculated from:

    sc.srv.initialStreamRecvWindowSize() - st.inflow.available()

Where (*flow).available is defined as:

    func (f *flow) available() int32 {
    	n := f.n
    	if f.conn != nil && f.conn.n < n {
    		n = f.conn.n
    	}
    	return n
    }

When f.conn.c < f.n, it gets a bigger increment size. It should be
calculated from:

    sc.srv.initialStreamRecvWindowSize() - st.inflow.n

While we're here, remove an unnecessary type conversion too.

Updates golang/go#56315.

Change-Id: I4b26b27e4c5c5cd66e6a32b152d68f304adc65d8
GitHub-Last-Rev: 02fc09c1e7564243630d3555e7880ba352eea0fe
GitHub-Pull-Request: golang/net#155
Reviewed-on: https://go-review.googlesource.com/c/net/+/444816
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Run-TryBot: Damien Neil <dneil@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ZekeLu @gopherbot