Skip to content
This repository has been archived by the owner on Jan 13, 2021. It is now read-only.

Performance Tuning #56

Open
3 of 5 tasks
Lukasa opened this issue May 20, 2014 · 18 comments
Open
3 of 5 tasks

Performance Tuning #56

Lukasa opened this issue May 20, 2014 · 18 comments

Comments

@Lukasa
Copy link
Member

Lukasa commented May 20, 2014

hyper is not optimised for performance right now. While the HTTP/2 spec is changing I want to focus on correctness and the ability to easily change behaviour.

However, when it does get nailed down we'll want to make some steps to improve performance. Roberto Peon has provided an awesome list of things to work on, which I've reproduced here. People should take things off this list and break them out into new issues as they go.

  • Rather than two calls to socket.read() per frame, we should read into a buffer and then parse the buffer. Avoids repeated kernel-userspace transitions.
  • Avoid copying the data around when framing. This is somewhat challenging.
  • Add optional support for nghttp2's C HPACK implementation. (Add optional support for nghttp2's HPACK implementation #60)
  • Use memoryview objects in hyper's pure-Python HPACK implementation. (Use memoryviews in HPACK #61)
  • We can definitely improve the Huffman implementation. Consider using this and this for ideas. Alternatively, consider arranging the Huffman table by length an then use one bit at a time, considering only the characters of the appropriate length.

Other intelligent performance optimisations should be added here.

@Lukasa
Copy link
Member Author

Lukasa commented May 20, 2014

The first two points should provide a huge performance boost if handled appropriately.

@schlamar
Copy link

schlamar commented Jun 4, 2014

I just wanted to come here and propose point number 1 after reading your noteboook and then found out that you are already thinking about it :) I can confirm that reading a block into a buffer is a significant improvement on a proprietary TCP protocol with sized framing.

Right now the buffer is a simple string which gets expanded after socket.recv. But there might be more efficient alternatives. Maybe socket.recv_info can be used with a StringIO or something like that. Do you have some ideas for an efficient buffer implementation? Anything that helps with point two would be great. :)

@dimaqq
Copy link

dimaqq commented Jun 4, 2014

On a related note, please be more thorough than time.time(), for example:

if sys.platform.lower().startswith("linux"):
    def _rusage():
        tmp = resource.getrusage(1)  # RUSAGE_THREAD
        return dict(time=time.time(),    # wall
                    utime=tmp.ru_utime,  # user
                    stime=tmp.ru_stime,  # system
                    switch=tmp.ru_nivcsw * 1.,   # cpu contention
                    read=tmp.ru_inblock * 512.,  # disk io
                    write=tmp.ru_oublock * 512.,
                    fault=tmp.ru_majflt * 1.)    # memory contention

@Lukasa
Copy link
Member Author

Lukasa commented Jun 4, 2014

@dimaqq Good advice, though I was running the notebook on OS X, which would have limited the utility of that function. Might be worth me going back and adding it just for those who run on other platforms though, I'll have a think.

@schlamar I'm not yet sure, I don't know enough about efficient buffers in Python. Note sure that StringIO will work though, because I think the buffer you pass to the recv_into call needs to be a memoryview. You could have a bytearray and a memoryview to it, and use that. This would lead to, as an initial step, a very simple userspace-buffered socket.

More ideally we'd like to be able to use a fixed-size buffer as a ring buffer. I've never seen this done in Python, let-alone in pure Python, and I don't know that it's possible. I'll need to think about how I'd pull it off.

@sigmavirus24
Copy link
Contributor

@Lukasa do you really want a ring buffer? If I remember correctly, won't it write over old data in the buffer it isn't read quickly enough?

@Lukasa
Copy link
Member Author

Lukasa commented Jun 4, 2014

The general plan is no. =) socket.recv_into only reads in as much data as there is space in the buffer. It should in principle be possible to have that number be equivalent to the amount of space left in the actual ring buffer.

Of course, that's almost certainly impossible to do without C extensions, which I can't do. So it'll be easier to do as a single linear buffer.

@sigmavirus24
Copy link
Contributor

For what it's worth, I can think of a way to enforce that with a BytesIO buffer (thanks requests-toolbelt) if you're supposed to be reading bytes. (The same logic would work for a StringIO buffer too)

@Lukasa
Copy link
Member Author

Lukasa commented Jun 4, 2014

Sadly, I don't think you can use BytesIO with a memoryview, which means I can't use socket.recv_into, which means I can't avoid the overhead of an extra memory copy.

@dimaqq
Copy link

dimaqq commented Jun 4, 2014

guys, I think this socket reading conversation is moot.
http/2 is (supposedly) meant to be used over TLS...

@Lukasa
Copy link
Member Author

Lukasa commented Jun 4, 2014

TLS still uses sockets. =)

@Lukasa
Copy link
Member Author

Lukasa commented Jun 4, 2014

It's also totally allowed to use HTTP/2 in plaintext. =)

@Lukasa
Copy link
Member Author

Lukasa commented Jun 15, 2014

The buffered socket idea has been implemented: see this diff.

I've also added some more optimisation ideas, contained in issues #60 and #61.

@sigmavirus24
Copy link
Contributor

Do we have any benchmarks showing the performance benefits? (Call me a pain if you will ;P)

@Lukasa
Copy link
Member Author

Lukasa commented Jun 15, 2014

Not yet, but I plan to re-run my big HTTP/1.1 - HTTP/2 comparison at some point to see how the numbers change.

@Lukasa
Copy link
Member Author

Lukasa commented Jun 15, 2014

In the meantime, pypy's test run speed got a big boost when I fixed the tests up, so anecdotally it 'feels' faster.

@sigmavirus24
Copy link
Contributor

I didn't doubt it was faster. I was just trying to pre-empt some people from finding this and complaining about lack of benchmarks.

@Lukasa
Copy link
Member Author

Lukasa commented Jun 15, 2014

Agreed, I'll try to get some numbers. =)

@Lukasa
Copy link
Member Author

Lukasa commented Aug 17, 2014

Support for nghttp2's HPACK implementation is now present.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants