lambda-fairy / maud

:pencil: Compile-time HTML templates for Rust
https://maud.lambda.xyz
Apache License 2.0
2.01k stars 132 forks source link

Write to an existing String buffer instead of allocating a new one #90

Open P-E-Meunier opened 7 years ago

P-E-Meunier commented 7 years ago

So, I'm trying to use maud for nest.pijul.com. So far I like it, but nest.pijul.com runs on exactly four threads, forked in the beginning, and is entirely nonblocking. More specifically:

At least one buffer per connection is not really avoidable if you don't want to mix requests between clients, but the internals of a server should not need that.

So, in this context, Maud has the potential to allocate a single buffer per thread, and reuse it between clients, because it is never interrupted by IO. In a standard synchronous server, I agree with your comments on benchmarks (you would need some number of buffers per client), but async IO can allow you to have a constant memory use and constant number of allocations.

lambda-fairy commented 7 years ago

Great to hear that you like it :)

Note that the Render trait has a .render_to() method, which appends to an existing buffer. The code generator uses this method by default, so there's nothing to worry about on that front as long as .render_to() itself is written efficiently.

The issue, then, would be with the html! macro itself. As you've noticed, every call to html! allocates a new buffer, which is less than optimal when you control the event loop. To solve this problem, we would need to either (a) take an output buffer as a parameter, or (b) wrap the generated code in a closure that does the same.

In my experience, either of these solutions will make Maud less ergonomic to use. I also think that for most users this trade-off is not worth it for them. (Maybe my opinion is wrong here, given that these users also chose Rust!) So if we do end up adding a no-allocation option for Maud, I think it should at least be optional.

lambda-fairy commented 7 years ago

I wonder if we could use trait overloading to pick between the two approaches based on context. Something like this:

trait FromTemplate<F: FnOnce(&mut String)> {
    fn from_template(template: F) -> Self;
}

impl FromTemplate<F: FnOnce(&mut String)> for Markup {
    fn from_template(template: F) -> Markup {
        let mut buffer = String::new();
        template(buffer);
        PreEscaped(buffer)
    }
}

struct MarkupFn<F>(F);

impl FromTemplate<F: FnOnce(&mut String)> for MarkupFn<F> {
    // ...
}

impl Render<F: FnOnce(&mut String)> for MarkupFn<F> {
    // ...
}

I'm not sure how the Render impl for MarkupFn would work though, given that the former takes &self. I guess we can change the trait to move the value instead.


I'm also wary of breaking control flow. For example, currently we can do this:

fn query_database() -> impl Iterator<Item=Result<DbRow, DbError>> { ... }

let result = html! {
    @for entry in query_database() {
        li (entry?)
    }
};

where the ? operator will break out of the enclosing scope.

If we wrap the generated code in a closure then this pattern will no longer work.

P-E-Meunier commented 7 years ago

I've not looked at how maud is implemented, so this might be a stupid suggestion, but how about splitting the html! into two different macros?

For instance, html_with_buffer!(buffer, …) and html!(…).

either of these solutions will make Maud less ergonomic to use

I agree for the solution with closures, but not for buffers. The best crates I've ever used let me know where allocations are made. This is more ergonomic:

lambda-fairy commented 7 years ago

Yep, I think you have a good point there.

I'd be okay with adding a html_to!(buffer, ...) macro, once the dust settles on #92. (I prefer this name because it's consistent with render_to.)

lambda-fairy commented 7 years ago

Okay -- since #92 has landed now, I'll be happy to take a PR to implement an html_to! macro as described above. The relevant code is in maud_macros/src/lib.rs.

lambda-fairy commented 7 years ago

@P-E-Meunier I've changed the title of the issue -- does this sound like what you're looking for?

I haven't done much async I/O work in Rust so I want to confirm that this addresses your use case.

P-E-Meunier commented 7 years ago

Yes, it does address my use case (the goal is to allocate a single buffer for all pages served during the entire life of the process).

sanmai-NL commented 5 years ago

This could help lessen the need for https://github.com/lfairy/maud/issues/90, since it removes the need to mention a String type in https://github.com/lfairy/maud/blob/365d0ab956c8cf5db9d027c11b052d7e201e63d6/maud_macros/src/lib.rs#L50-L55.

badicsalex commented 1 year ago

Hi,

I found myself writing the following code:

impl<'a> maud::Render for MyLittleStructure {
   fn render(&self) -> maud::Markup {
        html!(
            .container {
                ... Non trivial html stuff ...
            }
        )
    }
}

Since I'm rendering something like 16000 divs, I worry that this is going to be a performance bottleneck sooner or later.

I'd love to write something like the following:

impl<'a> maud::Render for MyLittleStructure {
    fn render_to(&self, buffer: &mut String) {
        html_to!(buffer,
            .container {
                ... Non trivial html stuff ...
            }
        )
    }
}

Are you still accepting PRs for this feature? I think this could be done very easily with the current state of the codebase.