umbrella ticket to resolve iteration / read size / chunked encoding questions

This ticket is intended to aggregate previous discussion from #539, #589, and #597 about the default value of `chunk_size` used by `iter_content` and `iter_lines`.

cc @mponton @gwrtheyrn @shazow

Issues:
1. The default read size of `iter_content` is 1 byte; this is probably inefficient
2. Requests does not expose the ability to read chunked encoding streams in the "correct" way, i.e., using the provided octet counts to tell how much to read.
3. However, this would not be suitable as the default implementation of `iter_content` anyway; not all websites are standards-compliant and when this was tried it caused more problems than it solved.
4. The current default read size for `iter_lines` is 10kB. This is high enough that iteration over lines can be perceived as unresponsive --- no lines are returned until all 10kB have been read.
5. There is no "correct" way to implement `iter_lines` using blocking I/O, we just have to bite the bullet and take a guess as to how much data we should read.
6. There's apparently some nondeterminism in `iter_lines`, I think because of the edge case where a read ends between a `\r` and a `\n`.
7. `iter_lines` is backed by `iter_content`, which operates on raw byte strings and splits at byte boundaries. I think there may be edge cases where we could split the body in the middle of a multi-byte encoding of a Unicode character.

My guess at a solution:
1. Set the default `chunk_size` to 1024 bytes, for both `iter_content` and `iter_lines`.
2. Provide a separate interface (possibly `iter_chunks`) for iterating over chunks of pages that are known to correctly implement chunked encoding, e.g., Twitter's firehose APIs
3. We may need our own implementation of `splitlines` that is deterministic with respect to our chunking boundaries, i.e., remembers if the last-read character was `\r` and suppresses a subsequent `\n`. We may also need to build in Unicode awareness at this level, i.e., decode as much of the body as is valid, then save any leftover invalid bytes to be prepended to the next chunk.

Comments and thoughts are much appreciated. Thanks for your time!


:+1: for `iter_chunks` 


@slingamn thanks so much for stepping up with this ;)


I'm not sure using 1024 as the default size for `iter_lines` is appropriate, I think it should be 1 byte. Don't get me wrong, **I KNOW** how bad this is for performance. But I stick with my original reasoning: If someone iterates on `iter_lines`, lines should be processed as soon as they are available and there should be no chance of "hanging" with complete lines in the buffer. The interface is called `iter_lines` so it should do just that by default.

A knowledgeable developer that wants better performance could still decide to use a larger read size knowing that it could mean that the read process will hang until the specified amount is read or the stream closed and that some lines could come in late.

I love `requests` because it is fun and easy to use first, yet very powerful. This is why I think `iter_lines` should behave as expected even if it means poor performance. This decision should simply be documented. Newcomers will appreciate this (i.e. works as expected out of the box) while more experienced devs can still fiddle with the buffer size.

That said, `iter_content` should definitely have a larger default read size. The previous one was 10K if I remember correctly. The newly proposed one is 1K. I'm not sure which one make a better default.

+1 for `iter_chunks`.


I think I'm missing something but why must `iter_lines` hang when it has complete lines in the buffer? Why can't we scan for completed lines (`\n` or such) and yield it just the same even if we've read less than the default size?


@shazow Because the synchronous network read call will not return until the specified amount of bytes has been read or the stream is closed. Thus, is you use a 1K read size and the server returns lines averaging 100 bytes, you will have to wait for the server to return at least 11 lines before `iter_lines` return any lines and your Python code is executed. If one line is sent every minute, it will take 11 minutes before your code is even aware of the first line sent by the server.


Ah I see what you mean. For some reason I was thinking in local IO terms. Thanks for the clarification!

I wonder if there is any way to do it timeout-based, such that it will read as much as it can in N ms and process, loop.


@mponton these are good points.

Does anyone know what browsers typically do when reading streaming text data without chunked encoding?


I did a quick test out of curiosity. It seems to depend on the `Content-Type`. For `text/ascii`, each line is printed as it comes in without delay (Safari 6). If content type is unspecified or HTML, the browser waits for the server to close the connection before rendering the content.

In any case, I'm not sure we should compare and try to emulate a browser... We're "stuck" in synchronous I/O land, browsers aren't. We should implement what makes the most sense for `requests`.


I was poking around for something unrelated and there's quite an interesting implementation of a `readline` method inside the `socket` module. I haven't read it in detail yet, but it looks as though it reads byte-at-a-time in some cases and 8kB at a time in others.

It doesn't respect unicode or universal newlines, so we probably can't use it out of the box, but it looks like they put quite a bit of thought into it, so maybe some of the ideas are applicable.


It seems most of the discussion is to do with how iter_lines should work. Is there any reason for iter_content not to have a larger default chunk size? The performance hit was really hard for the 1 byte size before I figured out what was going on.


There's no reason (at least, for `iter_content`'s own default, as opposed to the default that `iter_lines` uses it with). I've been sort of reluctant to just dive in and try to change it, lest we trigger latent bugs (which is what happened the last time I changed a default).

To be honest, I think the typical use case should just be to read the entire body with the `.content` property. `iter_content` strikes me as a difficult interface to use correctly.


I had changed back to 10K (original value) in #589 but the travisbot failed the build because one of the unit tests' expected result was not the same with a chunk size > 1 (see https://github.com/kennethreitz/requests/pull/589#issuecomment-5538019 for details).

Kenneth, proceeded to close the issue saying `iter_content` now had a proper size. At the time I thought he had made the changes on his side would merge them. Apparently this feel through a crack or something...


@slingamn Could you elaborate on point 3 from your initial list?


+1 for improving the default performance of iter_content, or at least, in the property Request.content, make sure to call iter_content with a larger chunk size. I think there's no reason for reading in chunks over there...

Edit: sorry. I see this has already been fixed in the latest release. Thanks for that!


@slingamn pointed out that there are a few issues here that are still unresolved. Sometime today I'll go through and work out which ones still haven't been done.


Current status as of v1.1.0:
1. the iter_content read size is still at 1 byte
2. the iter_lines read size has been reduced to 512 bytes (which should help responsiveness)
3. multibyte characters and newlines can still overlap with read boundaries
4. there's an implementation of chunked encoding for outgoing requests, but I'm not sure if there is one for incoming requests (the documentation says so, but I can't find the implementation)


My view:
1. Yeah, probably. Something small-ish though, as we generally expect users who need `iter_content` to specify their own values.
2. I'd call that fixed. =D
3. This has all the hallmarks of being an interesting problem. =) If no-one else does I'll probably take a look at seeing if we can fix this.
4. Again, interesting problem for someone to work on.


3 is an interesting problem. There is probably a minimum length you might want to consider in those cases, but I'm sure you won't be able to please everyone. Since UTF-8 is on the rise, we could probably use that as a way of deciding on a default length.


@slingamn #4's a documentation problem and needs to be corrected.


I would like to jump in because it appears that a bit of the above discussion is based upon a false premise: that a synchronous network receive of _n_ bytes will block until at least _n_ bytes are received (or, presumably, the socket has closed).

This is not, in fact, the case, and Unix network programming would be a shambles if it were — think of the disaster that every network program would face: applications would have to make the horrible decision to either read data byte-by-byte, or block indefinitely if they overestimated, even very slightly, the amount of data about to arrive on a socket. Network programmers would all stand impaled upon the horns of a dilemma. Everyone might use Windows for network programming instead. :)

But, fortunately, the plain normal vanilla blocking synchronous version of the `recv()` call is merciful to us: it only blocks waiting as long as _no data at all_ is ready to be consumed! As soon as _even a single byte_ of data arrives in an incoming packet, your beefy call to `recv(1048576)` will return _that single byte_ and your program is off and running again. There is _no penalty_ for giving `recv()` permission to return lots of data if a lot of data arrives in a single packet, or arrives in several packets while the operating system is still getting around to scheduling your thread again.

It is true that, for those rare exceptional cases where you really _want_ to stay blocked because you really know that you need _n_ bytes before you can do anything useful, there exists a POSIX flag `MSG_WAITALL` that you can pass to `recv()` so that your program really does block until lots of data arrives. But that is a very rare flag to specify, for all of these reasons that we have put on the table here.

So what is the problem here, you ask?

Well, @mponton actually lets the cat out of the bag without knowing it! Look carefully at this phrase from his reply to @shazow:

“…the synchronous network read call…”

“Read” call? What? Who would do a `read()` call on a socket? While POSIX does allow `read()` and `write()` on sockets for those extremely rare cases where you pass a socket to a library that is only designed for talking to files, it's not something you would ever do in a network program—you would lock yourself up waiting for _n_ bytes to arrive even if less than _n_ bytes were available immediately! Who would do _that?_

Why, the author of `httplib`, of course!

Yes, that's right. Instead of simply sitting in a tidy standard `recv()` loop and slowly filling a buffer until an end-of-line is visible, the author of `httplib` **wraps the socket in a fake file-like object with `makefile()` and completely abandons all of the benefits of network programming under Unix!**

They gain a tiny bit of convenience — and maybe, way back when it was written, C-level performance? — by having a Python file-like object watch for the end-of-line character for them. But they **completely disabled the ability to stream live data from the network** by making this choice, probably because they were operating in an era when people read and wrote network payloads whole anyway.

I recommend that Requests move off of `httplib`. It does very, very little for you; it could be re-implemented much more simply with `recv()` and `send()` in a day or two, given the simplicity of HTTP. There is no reason why Python or Requests shouldn't have line-based or content-based iteration that returns the moment that enough data has arrived to satisfy the caller; you just need to move off of a broken implementation that wantonly imposes the semantics of the `read()` call on what is really a socket capable of doing `recv()` if you'll just ask nicely!


I need to go have dinner, but another quick note that I'll expand on later: there's also no reason that `readline()` should find UTF-8 a challenge, because the pair of bytes `\r\n` look exactly the same in UTF-8 — and since _every byte_ of a multi-byte character in UTF-8 has the high bit set, no multi-byte character or sequence of them could accidentally have `\r\n` sitting inside. So you just need `readline()` to call `recv()` getting raw bytes until you see the sequence `\r\n`, and _then_ pass the sequence of bytes — that you know already is a single line — to the decoder to be turned into a string.


I doubt we're prepared to ditch httplib (and thus urllib3) over `iter_lines()`.


@brandon-rhodes thanks very much, this was extremely illuminating.


urllib3 would be very very happy to ditch httplib. https://github.com/shazow/urllib3/issues/58

I'm thinking of trying to find some corporate sponsors for making this happen and diving in (alongside some other high-demand issues).


@shazow: I'm all for that! Let me know if you get off the ground with it and I'll do my best to help out.


So, the implementation of iter_content calls `read` directly:

``` python
        def generate():
            while 1:
                chunk = self.raw.read(chunk_size)
                if not chunk:
                    break
                yield chunk
            self._content_consumed = True
```

So we can presumably just call `recv` directly here, modulo an issue where `self.raw` may be a `CStringIO` instead of a socket if the content was prefetched. This wouldn't require eliminating httplib.


@slingamn There is a danger here: once the socket has been wrapped with a file-like object by `makefile()`, there is a chance that the stdio buffering will have read a bit _past_ the end of the first line and, after returning that line to you, will hold in its buffer a good bit of data that lay _after_ the line ending.

Usually this is no problem, as when you next issue `readline()` or `read()` the file buffering simply draws data from its buffer first before then turning to the file descriptor for more.

But if you try to go _around_ the stdio input buffering and read directly from the socket, you will be missing the chunk of data (if any) that had been read past the end of the line! Unfortunately, stdio provides no way to introspect a `FILE *` object and rescue the contents of its buffer without also probably inducing the punishment of a blocking `read()`. So once you have even for a moment used a file-like object to do your reading for you, you are committed: you cannot return to raw socket I/O.

It might be possible to craft your own file-like object; I found, for example, that `pywsgi.py` in `gevent` let me provide my own `rfile` object that implements `read()` and `readline()` on top of a socket, and the problem was solved.

It is also possible that you might want a way around the `FILE *` problem, or even a guarantee that no data will be trapped there. I am often wrong about such things, and you may see a way out where I have not yet.

But I think the whole issue is what lead the Python 3 people to ditch `FILE *` I/O entirely and write a new dedicated Python buffering subsystem directly atop POSIX system calls. If only we were working under Python 3, we might not even face these issues. Alas that all of our favorite toys keep us toiling under Python 2!


@brandon-rhodes `makefile()` is implemented entirely in Python (in 2.7 at least) and is a wrapper around the low-level socket object. It is not a FILE \* object so it has nothing to do with stdio. Its internal buffering is implemented using StringIO. It has a `readline()` method that does exactly what we need (i.e. use `recv()` to get data from the network into a buffer and return immediately when an EOL is encountered) for `iter_lines()` (minus unicode decoding).

@slingamn The file-like object in `Response.raw` is a `urllib3` `HTTPResponse` wrapping a `httplib` `HTTPResponse`. The later uses `makefile()` to wrap the socket object but does no expose that socket object and thus it is not possible to call `recv()` from a `Response` object (well, unless you dig deep, see below).

As an example, here's an ugly kludge that would use an "optimized" way to read lines from the network:

```
# Decoding stuff ignored, just an example...
def iter_lines(self, max_size=10000, decode_unicode=None):
    while 1:
        # raw = urllib3 HTTPResponse
        # raw._fp = httplib HTTPResponse
        # raw._fp.fp = socket file-like object returned by socket.makefile()
        line = self.raw._fp.fp.readline(max_size)
        # raw._fp.fp._sock is the actual socket where we could call recv()
        if not line:
            break
        yield line
```

Here's an example of the result when reading 10 lines generated at 5 seconds interval with a buffer size of 10K:

Original `iter_lines` (stuck in `read()` for 50 seconds):

```
01:06:14: This is line 1
01:06:14: This is line 2
01:06:14: This is line 3
01:06:14: This is line 4
01:06:14: This is line 5
01:06:14: This is line 6
01:06:14: This is line 7
01:06:14: This is line 8
01:06:14: This is line 9
01:06:14: This is line 10
```

"Fixed" `iter_lines` (properly streaming every 5 seconds):

```
01:06:20: This is line 1
01:06:22: This is line 2
01:06:27: This is line 3
01:06:32: This is line 4
01:06:37: This is line 5
01:06:42: This is line 6
01:06:47: This is line 7
01:06:52: This is line 8
01:06:57: This is line 9
01:07:02: This is line 10
```

I'm sorry if my previous comments on network I/O might have been overly simplistic, but in the current context, they seemed right enough.

@brandon-rhodes Is right, the proper way of fixing this would be to either use `recv()` directly or use the `makefile()`-generated file-like object's `readline()` (which does exactly what Brandon describes). Considering that `httplib`'s HTTPResponse class does not "officialy" expose the file-like object it uses, using it for our own purpose may not be a good idea. This would be solved if `urllib3` stopped using `httplib` and properly exposed the methods/properties we need to fix this issue correctly but this would require some surgery...


@mponton, you are AWESOME — thank you for stopping me from making this fix more work than was necessary for everyone! My information was outdated. The last time I read `makefile()`, it merely did this with the socket (here I'm quoting from Python 2.3, which happens to be on my hard drive):

```
((fd = dup(s->sock_fd)) < 0 || (fp = fdopen(fd, mode)) == NULL)
```

I am VERY happy that the modern `makefile()`, even in Python 2.7, has been spruced up and improved and does not impose on Requests all of the problems that I was wildly claiming.


Oh: and, I'd recommend that Requests go with @mponton's solution here and not worry about whether `httplib` “officially” exposes the object or not; the library at this point has been entombed within amber for more than a decade and I would be surprised, given how much code depends upon its every peculiarity, whether it will ever change again at this point. If Python 3.4 or 3.5 does ever invalidate the access to `_fp` then a more difficult solution could be constructed at that point.