Shouldn't render_into() use io::Write instead of core::fmt::Write?

[bbigras]

I might be missing something but wouldn't using io::Write allow to write into a file directly?

[djc]

Hmm, maybe. Askama templates revolve more around Strings than around writing bytes. Do you have a particular use case where the current API is awkward?

Here's a branch that implements this: https://github.com/djc/askama/commits/io-write. However, does regress our current set of benchmarks. I'm not sure whether that's an artefact of the benchmarks or whether it reflects a real-world slowdown -- anyway, that means it needs more investigation.

[nic-hartley]

@djc I have a pair of usecases where it's awkward: Writing to Files and TcpStreams.

Out of curiosity, what makes you say that Askama templates revolve around writing to strings? That implies the expected way to use Askama is writing a (potentially extremely large) rendered template into memory, then sending it to wherever it actually needs to go, rather than being able to send it directly. And even then, io::Write supports that usecase as well -- Vec<u8> implements Write, and can be quite cheaply converted into a String. (Not as cheaply as the other way around, but given that implementing io::Write saves a huge amount of memory in most cases and keeps you from having to do two passes, IMO that's worth it...)

To be clear, I'm not opposed to having a method to write to a fmt::Write. I just cannot for the life of me see why it's the only one, and especially not why it's the default choice.

[djc]

@nic-hartley I'm not at all opposed to it, but it probably shouldn't regress the string rendering performance. If you're interested, please take my branch and see if you can reproduce/fix the performance issue.

[nic-hartley]

@djc Leaving the existing code as-is won't regress anything (at least, barring some extremely funky trickery). io::Write to a String will almost always be slower, because it works in bytes and String needs to do a UTF-8 validity check -- have you tried doing it to a Vec<u8> instead? I can take a look once I have free time, though that link is broken and I don't see it listed on GitHub -- could you re-push it if you still have it locally?

[djc]

Hmm, nope -- I'm afraid I removed it. :( It was probably kind of old anyway.

[ghost]

I ran into this today while playing around and it was a little surprising, i think it's a bit more ergonomic to use io::Write just looking at the docs of core::fmt::Write :

This is similar to the standard library's io::Write trait, but it is only intended for use in libcore.

This trait should generally not be implemented by consumers of the standard library. The write! macro accepts an instance of io::Write, and the io::Write trait is favored over implementing this trait.

I started playing around with the crate today and im prototyping some stuff with some data that i have, looking at the docs i kind of expected to just be able to do Html.render_into(&mut file)?;

instead i had to do

use std::io::Write;
file.write_all(html.render()?.as_bytes())?;

and although it's not too much more code overall it was a little surprising

I'd be happy to look into this a bit more and the regressions that you mentioned, do you have any ideas or advice?

[djc]

Not really, it's been too long since I last looked into it. Happy to answer any questions you might have, though!

[cipriancraciun]

I've also stumbled upon this issue, and my current solution is this: create an io::Write to fmt::Write adapter as bellow:

pub struct RenderIoFmtAdapter <'a, Stream : io::Write> {
	stream : &'a mut Stream,
}

impl <'a, Stream : io::Write> fmt::Write for IoFmtAdapter<'a, Stream> {
	
	fn write_str (&mut self, _string : &str) -> (Result<(), fmt::Error>) {
		match self.stream.write_all (_string.as_bytes ()) {
			Ok (()) =>
				return Ok (()),
			Err (_) =>
				return Err (fmt::Error),
		}
	}
	
	fn write_fmt (&mut self, _arguments : fmt::Arguments<'_>) -> (Result<(), fmt::Error>) {
		// FIXME:  Find a better (?) way to handle this!
		match write! (self.stream, "{}", _arguments) {
			Ok (()) =>
				return Ok (()),
			Err (_) =>
				return Err (fmt::Error),
		}
	}
}

It seems to work, and except the FIXME above (where I'm unsure efficiently render an fmt::Arguments into an io::Write) it shouldn't be slower than an alternative implementation that natively renders directly into an Vec<u8>.

@djc Would you accept a patch on the following lines:

• I add an WriterIoToFmt adapter based on the code above;
• add a method to the Template trait similar to render, named write_into which has the same signature like the render_into except that it accepts an io::Write;

The only tricky part, which isn't captured in my snippet above is how to handle errors:

• if they are io::Error generated by the undelying io::Write I'll catch them in a field of the adapter;
• if they are fmt::Error I'll generate an io::Error with the reason "other" and a generic message; (apparently fmt::Error doesn't have any extra details).

[djc]

I think I'd prefer to just rip out the fmt::Write layer in favor of io::Write.

[cipriancraciun]

I think I'd prefer to just rip out the fmt::Write layer in favor of io::Write.

Given the fact that askama is mainly a "text" template engine, I think that in fact fmt::Write makes more sense than io::Write.

Moreover as you've yourself stated it reduces the performance, most likely due to the str -> u8 conversions (although that shouldn't be the case given that a str can be casted to a &[u8] with zero cost).

[djc]

Hmm, fair enough -- if you write a patch, I'll review it to see whether I think it makes sense.

[nic-hartley]

The fact that it operates on text doesn't really change how that text is used, which should be the main decider -- is it mostly used to create Strings which are then moved around in memory? Or is it mostly used to generate data which is being sent places, e.g. down TcpStreams or into files? (Genuine question. I suspect the latter is vastly more common and the former barely exists, but you'd have to look at actual usage, maybe with crate reverse dependencies, to know for sure.)

[djc]

It seems clear to me that we can do both, and should ideally support both. Relying on strings as the primary interface is a bit more powerful because strings are more restrictive than byte sequences. As such, converting from strings to byte sequences is easier than the other way around, and this also counts for something.

[nic-hartley]

I'm not totally convinced that's possible without making the interface much harder to use, but if you think it's possible, I'll defer to that -- I have much less experience with the codebase, lmao

My main concern is that I might have very large templates and, in particular, template outputs, and I'd like to be able to stream those directly to files instead of having to build the whole thing up in memory, then dump it once at the end.

On Sun, May 17, 2020 at 4:55 PM Dirkjan Ochtman [email protected] wrote:

It seems clear to me that we can do both, and should ideally support both. Relying on strings as the primary interface is a bit more powerful because strings are more restrictive than byte sequences. As such, converting from strings to byte sequences is easier than the other way around, and this also counts for something.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/djc/askama/issues/163#issuecomment-629859295, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABBBR4AQA5NIR6PM27QI3ZDRSBFMVANCNFSM4GGS62HQ .

[cipriancraciun]

My main concern is that I might have very large templates and, in particular, template outputs, and I'd like to be able to stream those directly to files instead of having to build the whole thing up in memory, then dump it once at the end.

@nic-hartley based on my io::Write to fmt::Write adapter proposal, the streaming still happens, and no intermediary buffer is created.

My only "unknown" is how to efficiently format an fmt::Arguments in a io::Write without allocation. I would assume that my initial attempt write! (_stream, "{}", _arguments) would be a noop. (However in the worst case only a small allocation is made for that rendering, and not for the whole template.)

(I've yet to make the promised changes. Hopefully next weekend I'll manage to do so.) :(

[nic-hartley]

Makes sense! When I read "adapter", I was thinking some prebuilt code that does the same "read into string, send down pipe" wrapper that I did from the outside. If it actually adapts the calls so that it streams either way, I'm perfectly happy with an adapter.

I dug through some old code, btw, and found this -- I actually completely forgot about writing it. I'm fairly sure it's only good as a starting point, though. I seem to remember it just barely working because of how I'd set up some stuff around it?

use std::{fmt, io};

// thanks, stephaneyfx:
https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=fc473d7fffb1cb07e8e2c6b1dad65ced
pub struct WriteAdapter<W>(W);

impl<W> WriteAdapter<W> {
    // pub fn unadapt(self) -> W {
    //     self.0
    // }
}

impl<W: io::Write> fmt::Write for WriteAdapter<W> {
    fn write_str(&mut self, s: &str) -> Result<(), fmt::Error> {
        self.0.write_all(s.as_bytes()).map_err(|_| fmt::Error)
    }
}

impl<W: fmt::Write> io::Write for WriteAdapter<W> {
    fn flush(&mut self) -> io::Result<()> {
        Ok(())
    }

    fn write(&mut self, buf: &[u8]) -> io::Result<usize> {
        let s = std::str::from_utf8(buf)
            .map_err(|e| io::Error::new(io::ErrorKind::InvalidData, e))?;
        self.0
            .write_str(s)
            .map_err(|e| io::Error::new(io::ErrorKind::Other, e))?;
        Ok(s.len())
    }
}

pub fn adapt<W>(from: W) -> WriteAdapter<W> {
    WriteAdapter(from)
}

(Apologies in advance if that renders poorly -- I'm interacting with GH through email and I have no idea whether it'll play nice.)

On Tue, May 19, 2020, 16:07 Ciprian Dorin Craciun [email protected] wrote:

My main concern is that I might have very large templates and, in particular, template outputs, and I'd like to be able to stream those directly to files instead of having to build the whole thing up in memory, then dump it once at the end.

@nic-hartley https://github.com/nic-hartley based on my io::Write to fmt::Write adapter proposal, the streaming still happens, and no intermediary buffer is created.

My only "unknown" is how to efficiently format an fmt::Arguments in a io::Write without allocation. I would assume that my initial attempt write! (_stream, "{}", _arguments) would be a noop. (However in the worst case only a small allocation is made for that rendering, and not for the whole template.)

(I've yet to make the promised changes. Hopefully next weekend I'll manage to do so.) :(

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/djc/askama/issues/163#issuecomment-631053676, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABBBR4DAB7SKJSHOJHC3NTLRSLRIDANCNFSM4GGS62HQ .

[cipriancraciun]

@nic-hartley thanks for the input. The adapter you've mentioned is from io::Write to fmt::Write, and for our use-case we need the reverse from fmt::Write to io::Write.

(BTW, just today I had to implement a similar adapter like yours for writing JSON into an existing String. I'm quite amazed that there isn't in the library the adapters for these two...)

[nic-hartley]

@cipriancraciun should go both ways, actually.

I'm surprised there isn't one either. Seems like a common task. Might even make sense to have on in the standard library.

[cbzehner]

I just ran into this as well when trying to use render_into() and it took me a while to realize that uses std::fmt::Write while BufWriter<std::fs::File> uses std::io::Write...

error[E0277]: the trait bound `BufWriter<std::fs::File>: std::fmt::Write` is not satisfied
  --> src/template.rs:20:26
   |
20 |     template.render_into(&mut file_buffer);
   |              ----------- ^^^^^^^^^^^^^^^^ the trait `std::fmt::Write` is not implemented for `BufWriter<std::fs::File>`
   |              |
   |              required by a bound introduced by this call
   |
note: required by a bound in `render_into`
  --> /Users/cbzehner/.cargo/registry/src/github.com-1ecc6299db9ec823/askama-0.11.1/src/lib.rs:82:46
   |
82 |     fn render_into(&self, writer: &mut (impl std::fmt::Write + ?Sized)) -> Result<()>;
   |                                              ^^^^^^^^^^^^^^^ required by this bound in `render_into`

For more information about this error, try `rustc --explain E0277`.

I worked around it with an unoptimized approach for buffered writing of arbitrary structs implementing askama::Template to arbitrary files. I originally had template.render_into(&mut file_buffer); instead of using the intermediate template_str variable.

/// Write an Askama template to a file
pub(crate) fn write_template<T: askama::Template, P: AsRef<Path>>(template: T, path: P) -> color_eyre::Result<()> {
    let file = File::create(path)?;
    let mut file_buffer = BufWriter::new(file);

    let template_str = template.render()?;
    write!(file_buffer, "{}", &template_str)?;

    file_buffer.flush()?;
    Ok(())
}

PS: Not expecting a response here, just wanted to leave this as a comment in case it's helpful for future readers.