maud icon indicating copy to clipboard operation
maud copied to clipboard

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

Open P-E-Meunier opened this issue 7 years ago • 9 comments

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:

  • My async Postgres crate starts exactly one connection to the server per thread, i.e. currently four connections, independent from the number of clients or requests. It allocates two buffers per thread, once, and never reallocates or drops them.
  • My async HTTP server allocates two buffers per connection, and reuses them if you use HTTP keep-alive.
  • An SSH server allocating a constant number of buffers (like 8) per connection.

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.

P-E-Meunier avatar Jun 25 '17 11:06 P-E-Meunier

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 avatar Jul 02 '17 10:07 lambda-fairy

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.

lambda-fairy avatar Jul 02 '17 11:07 lambda-fairy

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:

  • the small short-term loss in ergonomy is not clear to me, since I usually start with a working example of calls to a crate, that I modify and expand. I believe most people do the same, which is why cargo doc encourages you to show examples.

  • in the long run, this actually makes the library more ergonomic, since you spend less time profiling and optimising when you know you can have full control. Also, more control with Rust's safety guarantees tend to make me feel more powerful (that might be just me, but I've heard similar reports by others, for instance about tokio).

P-E-Meunier avatar Jul 02 '17 12:07 P-E-Meunier

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 avatar Jul 08 '17 02:07 lambda-fairy

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 avatar Jul 30 '17 09:07 lambda-fairy

@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.

lambda-fairy avatar Jul 30 '17 09:07 lambda-fairy

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).

P-E-Meunier avatar Jul 30 '17 10:07 P-E-Meunier

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.

sanmai-NL avatar Oct 15 '18 13:10 sanmai-NL

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.

badicsalex avatar Nov 05 '22 21:11 badicsalex