djc / askama

Type-safe, compiled Jinja-like templates for Rust
Apache License 2.0
3.35k stars 215 forks source link

Make `|tojson` and few more filters faster #1008

Closed Kijewski closed 4 months ago

Kijewski commented 4 months ago

This proof-of-concept PR changes the filter |tojson so that it does not collect the serialized data into a string, but writes the data into the target writer directly.

This makes the benchmark run 81% (only serializing) or 39% (serializing and escaping for HTML) faster. The benchmarked data is not a fair representation for the data you would most likely escape, though.

Kijewski commented 4 months ago

Also, I think the default |tojson filter should not prettify the JSON data. I guess it's unlikely that you want to show a JSON representation to the user, and it's much more likely that you want to fill some Javascript variable, never to be seen by the user.

djc commented 4 months ago

Also, I think the default |tojson filter should not prettify the JSON data. I guess it's unlikely that you want to show a JSON representation to the user, and it's much more likely that you want to fill some Javascript variable, never to be seen by the user.

What are the costs of making JSON pretty? I'm inclined to think the cost of JSON encoding will probably not show up for people and in most contexts pretty JSON will be helpful more than it will be harmful (because of increased size). So there's a trade-off:

  1. Performance cost of rendering pretty vs compact
  2. Bandwidth cost of transmitting pretty vs compact
  3. Developer experience benefits of rendering pretty vs compact

I would argue that (3) is likely to outstrip (1) and (2), but obviously it depends on the situation.

(To be clear, I'm arguing that you might want to look at your HTML/JS source code sometimes and when you do, it will be nice if you can follow the general structure of the JSON code, even if the JSON is not shown to the actual user.)

Kijewski commented 4 months ago

Let's keep |tojson pretty for now, and maybe discuss in a new issue if we should change it later?

Kijewski commented 4 months ago

So, I let all filters return impl fmt::Display. This has the added advantage that we can change the implementation of e.g. |trim without it being a semver incompatible change.