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

Jump to bottom

HTTP/2 downloads slower with default flow control #1102

Closed
afrind opened this issue Nov 3, 2016 · 9 comments
Closed

HTTP/2 downloads slower with default flow control #1102

afrind opened this issue Nov 3, 2016 · 9 comments
Labels
HTTP/2 known-issue

Comments

@afrind
Copy link

afrind commented Nov 3, 2016 •
edited by jay

Downloading a large object is much slower using HTTP/2. I peeked at the code and I'm pretty sure curl is using the default flow control window (65k). This is pretty small (I think Chrome opens the window to 10MB? by default). I didn't see an option to use a larger flow control window. I suggest that curl use a larger buffer by default and provide an option to use a larger one. Here's an example of pulling a 23MB image with a 33ms RTT:

HTTP/1.1

$ time curl  https://upload.wikimedia.org/wikipedia/commons/d/d6/Seattle_7.jpg >/dev/null --http1.1
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100 23.0M  100 23.0M    0     0  21.1M      0  0:00:01  0:00:01 --:--:-- 21.1M

real	0m1.101s
user	0m0.084s
sys	0m0.113s

HTTP/2

$ time curl  https://upload.wikimedia.org/wikipedia/commons/d/d6/Seattle_7.jpg >/dev/null 
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100 23.0M  100 23.0M    0     0  2858k      0  0:00:08  0:00:08 --:--:-- 2945k

real	0m8.283s
user	0m0.111s
sys	0m0.097s

curl -V

curl 7.49.0 (x86_64-redhat-linux-gnu) libcurl/7.49.0 NSS/3.19.1 Basic ECC zlib/1.2.7 libidn/1.28 libpsl/0.7.0 (+libicu/50.1.2) libssh2/1.4.3 nghttp2/1.7.1
Protocols: dict file ftp ftps gopher http https imap imaps ldap ldaps pop3 pop3s rtsp scp sftp smb smbs smtp smtps telnet tftp 
Features: AsynchDNS IDN IPv6 Largefile GSS-API Kerberos SPNEGO NTLM NTLM_WB SSL libz HTTP2 UnixSockets Metalink PSL 

CentOS Linux release 7.2.1511 (Core) 
Linux 4.0.9-68_fbk12_4058_gd84fdb3 #1 SMP Fri Aug 26 15:02:40 PDT 2016 x86_64 x86_64 x86_64 GNU/Linux
@bagder
Copy link
Member

bagder commented Nov 3, 2016

Correct, we should start with fixing the h2 window use as mentioned in the TODO.

@jay
Copy link
Member

jay commented Nov 15, 2016

I can reproduce with that website but I tried retrieving the same file from a local server Apache/2.4.18 w/nghttp2 and the difference isn't significant it averaged .004s real faster for http1.1. Since it is local I guess that's to be expected. How do we do this, if I change NGHTTP2_SETTINGS_INITIAL_WINDOW_SIZE to 1 << 30 I don't see any change in speed.

/cc @tatsuhiro-t

@tatsuhiro-t
Copy link
Contributor

The delay is caused by the latency between the moment sender blocked due to window size and the moment it receives WINDOW_UPDATE. The local server (loopback?) has low latency, so the difference is not significant.

I think you should also enlarge connection level flow control window by calling nghttp2_session_set_local_window_size(session, NGHTTP2_FLAG_NONE, 0, 1 << 30) after nghttp2_submit_settings().

jay added a commit to jay/curl that referenced this issue Nov 16, 2016
@jay
http2: Use huge HTTP/2 windows. draft1
ee90b6d 
- Improve performance by using a huge HTTP/2 window.

Bug: curl#1102
Reported-by: afrind@users.noreply.github.com
Assisted-by: Tatsuhiro Tsujikawa
@jay
Copy link
Member

jay commented Nov 16, 2016

I think you should also enlarge connection level flow control window by calling nghttp2_session_set_local_window_size(session, NGHTTP2_FLAG_NONE, 0, 1 << 30) after nghttp2_submit_settings().

Yup that did it, thanks. It only works if I do both. Here is the first draft https://github.com/curl/curl/compare/master...jay:use_large_http2_window?expand=1

@bagder
Copy link
Member

bagder commented Nov 16, 2016

Looks good to me!

@tatsuhiro-t
Copy link
Contributor

Looks good to me too.

jay added a commit that referenced this issue Nov 16, 2016
@jay
http2: Use huge HTTP/2 windows
a4d8888 
- Improve performance by using a huge HTTP/2 window size.

Bug: #1102
Reported-by: afrind@users.noreply.github.com
Assisted-by: Tatsuhiro Tsujikawa
@jay
Copy link
Member

jay commented Nov 16, 2016

Thanks guys. I just landed this in a4d8888 from a second draft, which has these changes from draft1:

  • Use a define constant HTTP2_HUGE_WINDOW_SIZE for the huge window size so if it needs to be changed it will affect both window size settings. In draft1 I just used 1 << 30 two places.
  • Set the connection window size always in Curl_http2_switched. In draft1 I was not setting it for upgraded connections.
  • Remove the TODO item. In draft1 I wrote FIXED but I think if it's fixed it should be removed.

@jay jay closed this as completed Nov 16, 2016
@bagder
Copy link
Member

bagder commented Nov 16, 2016

Great work @jay, I've wanted to see this done for a while but I never manged to pull myself together to get it done. Now you did it. Thanks!

@afrind
Copy link
Author

afrind commented Nov 16, 2016

Thanks all!

From: Daniel Stenberg notifications@github.com
Reply-To: curl/curl reply@reply.github.com
Date: Wednesday, November 16, 2016 at 2:49 PM
To: curl/curl curl@noreply.github.com
Cc: Alan Frindell afrind@fb.com, Author author@noreply.github.com
Subject: Re: [curl/curl] HTTP/2 downloads slower with default flow control (#1102)

Great work @jayhttps://github.com/jay, I've wanted to see this done for a while but I never manged to pull myself together to get it done. Now you did it. Thanks!

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHubhttps://github.com//issues/1102#issuecomment-261098180, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AH4IabHpjLD2vFXT00Ii4ol46TtOWd9sks5q-4iKgaJpZM4KonU9.

@lock lock bot locked as resolved and limited conversation to collaborators May 7, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
HTTP/2 known-issue
Milestone
No milestone
Development

No branches or pull requests
4 participants
@bagder @tatsuhiro-t @jay @afrind