Using yield from for sync cases

[michaeloliverx]

Take the following code:

httpcore/httpcore/_sync/connection_pool.py

Lines 345 to 347 in 1d90b36

	def __iter__(self) -> Iterator[bytes]:
	for part in self._stream:
	yield part

This can be written as:

    def __iter__(self) -> Iterator[bytes]:
        yield from self._stream

Sourcery's docs mention:

Eliminating the for loop also makes the yield from version about 15% faster.

There are a couple of places where this could be done:

httpcore/_models.py:434-435
httpcore/_sync/connection_pool.py:346-347
httpcore/_sync/http11.py:316-317
httpcore/_sync/http2.py:498-501

Unfortunately it is not compatible with the unasync.py script setup we have in place.

[karpetrosyan]

I believe we should change it for ConnectionPoolByteStream, but not for the HTTP11ConnectionByteStream and HTTP2ConnectionByteStream classes according to #715.

[lovelydinosaur]

That's certainly an interesting observation @michaeloliverx, thanks.
I wonder if we'd be able to concretely demonstrate the difference in overall performance?

There's a similar code change (with a different motivation) at #657

[lovelydinosaur]

Okay, and what do we think that actual impact for our end-users is here?
Does this end up as a rounding error vs. time spent in content decoding, or is there a noticeable gain that we might be able to make here?

Presumably if it is impactful then it'd be evident for large downloads.

[GalaxySnail]

Unfortunately the code by ChatGPT doesn't work. io.BytesIO(...) isn't an iterable object as the code expected:

>>> import io
>>> b = io.BytesIO(bytes(10000))
>>> it = iter(b)
>>> len(next(it))
10000
>>> len(next(it))
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
StopIteration

The correct benchmark looks like this (in IPython console):

In [1]: from collections.abc import Iterable

In [2]: def get_content_yield(stream: Iterable[bytes]):
   ...:     for content in stream:
   ...:         yield content
   ...:

In [3]: def get_content_yield_from(stream: Iterable[bytes]):
   ...:     yield from stream
   ...:

In [4]: import itertools

In [5]: def benchmark(func, total):
   ...:     it = itertools.repeat(bytes(4096), total)
   ...:     if func is not None:
   ...:         it = func(it)
   ...:     for _ in it:
   ...:         pass
   ...:

In [6]: %timeit -n 1000 benchmark(None, 128 * 1024)
1.4 ms ± 70.8 µs per loop (mean ± std. dev. of 7 runs, 1,000 loops each)

In [7]: %timeit -n 1000 benchmark(get_content_yield, 128 * 1024)
7.3 ms ± 147 µs per loop (mean ± std. dev. of 7 runs, 1,000 loops each)

In [8]: %timeit -n 1000 benchmark(get_content_yield_from, 128 * 1024)
6.77 ms ± 317 µs per loop (mean ± std. dev. of 7 runs, 1,000 loops each)

In [9]: %timeit benchmark(None, 256 * 1024)
3.08 ms ± 232 µs per loop (mean ± std. dev. of 7 runs, 100 loops each)

In [10]: %timeit benchmark(get_content_yield, 256 * 1024)
14.4 ms ± 683 µs per loop (mean ± std. dev. of 7 runs, 100 loops each)

In [11]: %timeit benchmark(get_content_yield_from, 256 * 1024)
12.3 ms ± 1.14 ms per loop (mean ± std. dev. of 7 runs, 100 loops each)

In [10]: %timeit -n 1000 benchmark(get_content_yield_from, 128 * 1024)
6.38 ms ± 205 µs per loop (mean ± std. dev. of 7 runs, 1,000 loops each)

In [11]: %timeit -n 1000 benchmark(get_content_yield_from, 128 * 1024)
6.39 ms ± 150 µs per loop (mean ± std. dev. of 7 runs, 1,000 loops each)

In [12]: %timeit -n 1000 benchmark(get_content_yield_from, 128 * 1024)
6.45 ms ± 214 µs per loop (mean ± std. dev. of 7 runs, 1,000 loops each)

[michaeloliverx]

Unfortunately the code by ChatGPT doesn't work. io.BytesIO(...) isn't an iterable object as the code expected:

>>> import io
>>> b = io.BytesIO(bytes(10000))
>>> it = iter(b)
>>> len(next(it))
10000
>>> len(next(it))
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
StopIteration

You need to do b.seek(0) after calling len(next(it)) to put the buffer back to the start.

Anyway it looks like you got some useful timings anyway!

[michaeloliverx]

Okay, and what do we think that actual impact for our end-users is here?
Does this end up as a rounding error vs. time spent in content decoding, or is there a noticeable gain that we might be able to make here?

I don't know how we would benchmark it for user code tbh, it might be negligible in the grand scheme of things but seems like an easy perf win if we could make it work with the unasync script.

[lovelydinosaur]

Hrm... a good starting point might be some real world benchmarks that we can work against?

I'd expect the classes of things we'd want to measure would be...

• Small GET downloads.
• Large GET downloads.
• Small POST uploads.
• Large POST uploads.
• Large number of concurrent requests.

Although we might only want to start off by considering one of those classes.

There's various work referenced by encode/httpx#838 that might be helpful for us.