-
-
Notifications
You must be signed in to change notification settings - Fork 34k
http2: tune control flow defaults #61036
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
base: main
Are you sure you want to change the base?
Conversation
|
Review requested:
|
|
I'm unsure about the relationship between request window size (initialWindowSize) and connection window size (connectionWindowSize). But I assume it's ok to have a 1:2 relationship. 🤷 |
|
This affects UPLOAD speed, i.e. data to server. |
| this.emit('session', session); | ||
| } | ||
|
|
||
| function initializeOptions(options) { | ||
| assertIsObject(options, 'options'); | ||
| options = { ...options }; | ||
| options = { ...getDefaultSettings(), ...options }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should actually apply the default settings and not just assume that they are the same as nghttp2 will do.
f6732e4 to
eae93cc
Compare
The current defaults are unnecessarily conservative which makes http2 control flow over high latency connections (such as public internet) unbearably slow.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #61036 +/- ##
==========================================
- Coverage 88.58% 88.47% -0.12%
==========================================
Files 704 703 -1
Lines 207815 208549 +734
Branches 40036 40199 +163
==========================================
+ Hits 184102 184517 +415
- Misses 15758 16027 +269
- Partials 7955 8005 +50
🚀 New features to boost your workflow:
|
mcollina
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
I think this might require a doc change too
The current defaults are unnecessarily conservative which makes http2 control flow over high latency connections (such as public internet) unbearably slow.