luckyframework / lucky

A full-featured Crystal web framework that catches bugs for you, runs incredibly fast, and helps you write code that lasts.
https://luckyframework.org
MIT License
2.59k stars 156 forks source link

Write html and json directly to response body instead of intermediary #1663

Closed matthewmcgarvey closed 2 years ago

matthewmcgarvey commented 2 years ago

Currently, Lucky::HTMLPage writes to an IO::Memory and then that is transferred over to the response body. Same goes for json, except it's turned to a string as its intermediary. This change adds functionality to write directly to the response body. In benchmarks I'm seeing the current code as 1.04 to 1.07 times slower. I'd love any ideas as to a better way to do this. I'm not too fond with this implementation.

Benchmark
```crystal require "benchmark" require "./spec/spec_helper" class BenchmarkPage include Lucky::HTMLPage def render header({class: "header"}) do style "body { font-size: 2em; }" text "my text" h1 "h1" br div class: "empty-contents" br({class: "br"}) br class: "br" img({src: "src"}) h2 "A bit smaller", {class: "peculiar"} h6 class: "h6" do small "super tiny", class: "so-small" span "wow" end end end end Benchmark.ips do |x| x.report("rewrite to output io") do output = IO::Memory.new io = BenchmarkPage.new(ContextHelper.build_context).perform_render output.print(io) end x.report("write directly to output") do output = IO::Memory.new BenchmarkPage.new(ContextHelper.build_context).view(output).perform_render end end ```
Benchmark Result
```bash ❯ crystal run benchmark.cr --release rewrite to output io 24.11k ( 41.48µs) (± 1.41%) 40.9kB/op 1.04× slower write directly to output 24.97k ( 40.05µs) (± 3.04%) 38.8kB/op fastest ```
robacarp commented 2 years ago

These are both so fast and roughly equivalent, I don't think I see the merit in refining the code for performance purposes.

matthewmcgarvey commented 2 years ago

While I guess the speed wouldn't change that much in a real app, I'd expect the memory to be different than this shows. My benchmark is still storing the string in memory but this change actually wouldn't do that since it's writing directly to the response body

jwoertink commented 2 years ago

@matthewmcgarvey are you able to run that benchmark using this https://crystal-lang.org/api/1.3.2/Benchmark.html#memory%28%26%29-instance-method ?

matthewmcgarvey commented 2 years ago

This doesn't seem to be the easy optimization I thought it could be. In some tests I'm actually seeing it cause a slowdown. I don't know why