vapor / http

🚀 Non-blocking, event-driven HTTP built on Swift NIO.
MIT License
238 stars 65 forks source link

fix unnecessary body copying #346

Closed tanner0101 closed 5 years ago

tanner0101 commented 5 years ago

This PR fixes an issue that causes HTTP request body byte buffer to reallocate unnecessarily. During large file uploads, this issue quickly consumes the CPU.

Screen Shot 2019-03-13 at 9 23 33 PM

The reason for the copy is that HTTPServerState continues to hold a reference to the HTTP request ByteBuffer when the new chunk is written. Since the buffer is not uniquely referenced, the write triggers a copy.

To fix this, the request body buffer is moved outside of the state enum to a member var. This ensures the ByteBuffer is uniquely referenced during body collection.

penny-coin commented 5 years ago

Hey @tanner0101, you just merged a pull request, have a coin!

You now have 1136 coins.

calebkleveter commented 5 years ago

Is it possible to write a test case to make sure this doesn't happen again? Maybe just a performance test would do.

tanner0101 commented 5 years ago

@calebkleveter yeah I think a performance test for a large file upload would do the trick. I'll open an issue.