ssr-rs icon indicating copy to clipboard operation
ssr-rs copied to clipboard

WIP: ssr context persistence

Open ryanswrt opened this issue 2 years ago • 3 comments

Proof of concept approach as discussed https://github.com/Valerioageno/ssr-rs/pull/1#issuecomment-994671463 to maintaining the context - a bit silly and and leaves a spinning future so has to be reworked quite a bit; but performance increases by about an order of magnitude (per actix-web logger response times - pretty much in line with the time taken to serve the static assets);

Before: 0.006800 After: 0.000531

ryanswrt avatar Dec 16 '21 10:12 ryanswrt

Hey Ryan, nice start! I knew that the performance increasing would have been huge! And obviously thank you! I was wondering if use concurrency directly in the library is the right way. Indeed it seems right so!

Valerioageno avatar Dec 20 '21 20:12 Valerioageno

Yeah, I'm not sure what the best practice would be for spawning multiple v8 workers - was wondering if Deno or something would have examples but they abstract at a different level.

Could probably move the worker thread spawner and expose an arc mutex channel to reduce the boilerplate (not sure why actix is fine with sharing the Ssr struct but all the other frameworks complain).

Also the changes to the client should probably be in a separate PR - increasing the speed and load testing exposed some bugs that are present in the current version... At what point would you be happy to merge this?

ryanswrt avatar Dec 21 '21 02:12 ryanswrt

Alright I've performed some tests but your integration is the fastest anyway. I don't think implement the arc mutex inside the library is good since I prefer leave to the final dev the choice about how to implement it even if it require some boilerplate (what do you think?). I'd like to also leave a shot() function anyway like the current render_to_string(). I was thinking about other kind of implementation and it could be worth try to create a JS snapshot with v8::SnapshotCreator::new() and then call it from the request function or share the isolate (rusty_v8 team is working on it PR) when this functionality will be available.

I'd like to merge your PR and publish on crates.io when it runs on all already implemented server example and pass the tests. What bugs are you referring to?

No problem to leave the client update on this PR.

Thanks Ryan

Valerioageno avatar Dec 21 '21 20:12 Valerioageno