search

thruster-rs / Thruster

A fast, middleware based, web framework written in Rust
MIT License
1.06k stars 47 forks source link

Serving of moderate size assets > 1MiB super slow #118

Closed xacrimon closed 5 years ago

xacrimon commented 5 years ago

As the title says responses with significant size take a lot of time such that i can't get more than 300 requests per second for a 1MiB image that is cached in ram.

trezm commented 5 years ago

I'll take a look into this!

trezm commented 5 years ago

Are you able to provide some sample code as well?

xacrimon commented 5 years ago

Of course! The service which had this problem has since been rewritten using actix-web until async await drops. Thanks to git history however it is still available here https://gitlab.nebulanet.cc/xacrimon/dubu-api/tree/28169c09171f9136d48374e2f07ae5769f72d276

trezm commented 5 years ago

After looking into this at a surface level, it seems like responses are scaling proportionately with the size of the payload (plus a constant,) what are you expecting/comparing these stats to? If another framework (actix I imagine,) is providing consistently better results, I can check into what they're doing differently!

xacrimon commented 5 years ago

I'd imagine this is because of the underlying HTTP impl. Yes I am comparing it to actix and hyper.

On Mon, Jul 15, 2019, 00:32 Peter Mertz notifications@github.com wrote:

After looking into this at a surface level, it seems like responses are scaling proportionately with the size of the payload (plus a constant,) what are you expecting/comparing these stats to? If another framework (actix I imagine,) is providing consistently better results, I can check into what they're doing differently!

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/trezm/Thruster/issues/118?email_source=notifications&email_token=AFANDB6MHTGG4BDHUYFJDYLP7OSRTA5CNFSM4IDR4XLKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODZ4OURQ#issuecomment-511240774, or mute the thread https://github.com/notifications/unsubscribe-auth/AFANDB2UBBWGCZTH6MCNSPDP7OSRTANCNFSM4IDR4XLA .

xacrimon commented 5 years ago

Probably due to how responses are actually being sent. Maybe streaming or something of the like.

trezm commented 5 years ago

That was my initial assumption as well, as the basic server implementation uses a framed response. Using hyper as an underlying mechanism is also an option, let me see if that tweaks the performance a significant amount.

xacrimon commented 5 years ago

Sounds good!

trezm commented 5 years ago

Alrighty, there's still some more long term work to do, but I got about a 20% speedup for the time being! Check out #119 for more details

xacrimon commented 5 years ago

Did using hyper as a server speed it up? @trezm

trezm commented 5 years ago

Ah, good question. I'm actually having some trouble integrating hyper with the new async await. Lots of complaints around how hyper::body::Body can't be sent between threads.

xacrimon commented 5 years ago

Ah, same issues here. Was going to report it. Nothing in hyper is compatible here. Just with this frameworks. No problem with others. Hyper client doesn't work either.

On Tue, Jul 16, 2019, 13:55 Peter Mertz notifications@github.com wrote:

Ah, good question. I'm actually having some trouble integrating hyper with the new async await. Lots of complaints around how hyper::body::Body can't be sent between threads.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/trezm/Thruster/issues/118?email_source=notifications&email_token=AFANDB4FFAJFO5TQGQB5MVDP7WZJJA5CNFSM4IDR4XLKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD2ATNAI#issuecomment-511784577, or mute the thread https://github.com/notifications/unsubscribe-auth/AFANDB34O6UO4CKQJN3PUKTP7WZJJANCNFSM4IDR4XLA .

trezm commented 5 years ago

Hmm, that's interesting. I've had success using reqwest which is based on hyper using Thruster.

As for the Body, I'm sure there's a way to do it, the backend is just in a state of flux since it supports both legacy and nightly futures/async-await.

xacrimon commented 5 years ago

Reqwests sync works. It's async that causes issues.

On Tue, Jul 16, 2019, 14:09 Peter Mertz notifications@github.com wrote:

Hmm, that's interesting. I've had success using reqwest which is based on hyper using Thruster.

As for the Body, I'm sure there's a way to do it, the backend is just in a state of flux since it supports both legacy and nightly futures/async-await.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/trezm/Thruster/issues/118?email_source=notifications&email_token=AFANDB6PL5O7LR3PEROAB23P7W26JA5CNFSM4IDR4XLKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD2AUOOA#issuecomment-511788856, or mute the thread https://github.com/notifications/unsubscribe-auth/AFANDB2W7R2B32I7SGE6Y3LP7W26JANCNFSM4IDR4XLA .

trezm commented 5 years ago

Just checking in, definitely making some headway. Hopefully will have a working hyper/async combo in the next couple of days.

The issue, for posterity, is that we're storing the request stream on the Context. This requires that the stream be Sync so that the work can be shared. If I disable storing the request on the Context, it actually compiles fine (although it obviously isn't very useful since we don't have any data to read from.

I'll keep poking at it and see if I can find a workaround.

trezm commented 5 years ago

Alright, found the solution, merged in as of #119. You should now be able to use hyper on the backend, with pretty good results too! About 94% of the throughput of the Actix version, and that's with very little tweaking.

Also, you should now be able to use hyper as an http client if you need to make external requests.

xacrimon commented 5 years ago

Yay! Sounds good.

On Fri, Jul 19, 2019, 14:35 Peter Mertz notifications@github.com wrote:

Alright, found the solution, merged in as of #119 https://github.com/trezm/Thruster/pull/119. You should now be able to use hyper on the backend, with pretty good results too! About 94% of the throughput of the Actix version, and that's with very little tweaking.

Also, you should now be able to use hyper as an http client if you need to make external requests.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/trezm/Thruster/issues/118?email_source=notifications&email_token=AFANDB4VUZGW2SXD732TLBLQAGYIZA5CNFSM4IDR4XLKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD2LP7CY#issuecomment-513212299, or mute the thread https://github.com/notifications/unsubscribe-auth/AFANDB7X7M3IRNB3TDZ6JELQAGYIZANCNFSM4IDR4XLA .