Skip to content

Send content-length instead of chunked transfer#6

Open
alex wants to merge 2 commits into
masterfrom
content-length
Open

Send content-length instead of chunked transfer#6
alex wants to merge 2 commits into
masterfrom
content-length

Conversation

@alex
Copy link
Copy Markdown
Owner

@alex alex commented Dec 5, 2014

With this the results look like:

Python 2.7.6
go version go1.3.3 darwin/amd64
BENCH SOCKET:
   8GiB 0:00:19 [ 424MiB/s] [======================================================>] 100%
BENCH HTTPLIB:
   8GiB 0:00:22 [ 365MiB/s] [======================================================>] 100%
BENCH URLLIB3:
   8GiB 0:00:13 [ 601MiB/s] [======================================================>] 100%
BENCH REQUESTS
   8GiB 0:00:14 [ 562MiB/s] [======================================================>] 100%
BENCH GO HTTP
   8GiB 0:00:25 [ 320MiB/s] [======================================================>] 100%

Which is... unexpected

@alex
Copy link
Copy Markdown
Owner Author

alex commented Dec 5, 2014

I think content-length better matches the use case I'm interested in, but I don't want to merge this until I understand teh results: how the hheck are urllib3 and requests faster than just reading off a socket!

@Lukasa
Copy link
Copy Markdown

Lukasa commented Dec 5, 2014

Try making the socket buffered. Add the line s = s.makefile('rb').

@alex
Copy link
Copy Markdown
Owner Author

alex commented Dec 5, 2014

With:

diff --git a/bench_socket.py b/bench_socket.py
index 09f1c7d..4e27687 100644
--- a/bench_socket.py
+++ b/bench_socket.py
@@ -11,8 +11,9 @@ def main():
     s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
     s.connect(("localhost", 8080))
     s.sendall(b"GET / HTTP/1.1\r\n\r\n")
+    s = s.makefile('rb')
     while True:
-        sys.stdout.write(s.recv(CHUNK_SIZE))
+        sys.stdout.write(s.read(CHUNK_SIZE))


 if __name__ == "__main__":

applied, the results are:

Python 2.7.6
go version go1.3.3 darwin/amd64
BENCH SOCKET:
   8GiB 0:00:20 [ 398MiB/s] [======================================================>] 100%
BENCH HTTPLIB:
   8GiB 0:00:22 [ 362MiB/s] [======================================================>] 100%
BENCH URLLIB3:
   8GiB 0:00:14 [ 549MiB/s] [======================================================>] 100%
BENCH REQUESTS
   8GiB 0:00:15 [ 517MiB/s] [======================================================>] 100%
BENCH GO HTTP
   8GiB 0:00:26 [ 313MiB/s] [======================================================>] 100%

which don't look substantially different?

@Lukasa
Copy link
Copy Markdown

Lukasa commented Dec 5, 2014

Hmm, interesting. Seems requests and urllib3 are still magical.

We do set TCP_NODELAY. To my knowledge TCP_NODELAY shouldn't provide a performance boost in this case, but maybe...

@sigmavirus24
Copy link
Copy Markdown

As I recall, TCP_NODELAY is meant to improve for smaller writes being issued and to prevent the delay in receiving an ACK from the person reading from the socket. I'm forgetting but I think it can have a negative effect on large writes, but I would have to research that again

@kevinburke
Copy link
Copy Markdown
Contributor

if the horribly named TCP_NODELAY is on, the sender will send packets
immediately instead of potentially waiting for more data to come. the
effect is faster data transmission (sometimes) but potentially more
packets. if TCP_NODELAY is off, there will be fewer TCP packets but there
might be a delay before they are sent; we saw delay of up to 1 second I
believe.

as I understand it, if it's set in the client, it'd only really affect data
going from the client to the server. you'd need to set it in the server to
see an improvement in download data speed.

Kevin Burke
phone: 925.271.7005 | twentymilliseconds.com

On Fri, Dec 5, 2014 at 11:11 AM, Ian Cordasco notifications@github.com
wrote:

As I recall, TCP_NODELAY is meant to improve for smaller writes being
issued and to prevent the delay in receiving an ACK from the person reading
from the socket. I'm forgetting but I think it can have a negative effect
on large writes, but I would have to research that again


Reply to this email directly or view it on GitHub
#6 (comment).

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.

4 participants