Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Html view rendering performance #281

Closed
dv00d00 opened this issue Jun 28, 2018 · 3 comments
Closed

Html view rendering performance #281

dv00d00 opened this issue Jun 28, 2018 · 3 comments
Labels
ready for release Issue has been already resolved in the development branch

Comments

@dv00d00
Copy link
Contributor

dv00d00 commented Jun 28, 2018

I've added a stripped implementation of html view rendering to the techempower tests repo. It finished test run today. These are results https://www.techempower.com/benchmarks/#section=test&runid=3da523ee-fff1-45d8-9044-7feb532bf9ee&hw=ph&test=fortune&w=zik0zj-hr4e7z&a=2

name rps
giraffe-stripped 174,979
giraffe 41,136

While testing stripped version I observed that to render fortunes html the html generation itself produces around 100 KB allocation.

What I did is very straightforward, I just rendered html to the memory stream instead of relying on string concatenation as in current implementation, it surely can be optimized more. @gerardtoconnor has investigated this area deeply. Please take a look at this https://github.com/gerardtoconnor/ViewEngineTests/blob/master/TemplateViewEngine.fs

Fortunes tests were added in this PR TechEmpower/FrameworkBenchmarks#3863

@gerardtoconnor
Copy link
Member

Great work @dv00d00 !! I think your solution it great, in line with my ByteViewEngine & already very high up there in the rankings.

I'm looking into how we can juice more performance out of the framework, once that is done I can look into the templating a bit more but I think you're 90% there.

@dustinmoris
Copy link
Member

First of all congrats on these great results! Secondly I would love to get your changes from string concatenation to memory stream put into the current GiraffeViewEngine. A PR is more than welcome! Let me know if this is something you would like to work on or if it is up for grabs for somebody else!

Thanks a lot!

@dustinmoris dustinmoris added help wanted Community contribution or any kind of help much appreciated refactoring request Request to improve code without changing external APIs (e.g. perf improvements) PR approved A PR for this issue will get accepted (as long as inline with the comms) labels Aug 15, 2018
@dustinmoris dustinmoris added ready for release Issue has been already resolved in the development branch and removed PR approved A PR for this issue will get accepted (as long as inline with the comms) help wanted Community contribution or any kind of help much appreciated refactoring request Request to improve code without changing external APIs (e.g. perf improvements) labels Sep 13, 2018
@dustinmoris
Copy link
Member

Release is out: https://github.com/giraffe-fsharp/Giraffe/releases/tag/v3.0.0

Thanks for the great contributions! :) 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready for release Issue has been already resolved in the development branch
Projects
None yet
Development

No branches or pull requests

3 participants