Skip to content

Reconsider buffering in say #57

@dtolnay

Description

@dtolnay

ferris_says::say currently works like this:

pub fn say<W>(..., writer: W) -> Result<()> 
where 
    W: Write, 
{ 
    let mut write_buffer = SmallVec::<[u8; BUFSIZE]>::new(); 

    write_buffer.extend_from_slice(...);
    write_buffer.extend_from_slice(...);
    ...

    writer.write_all(&write_buffer)
}

This is not a pattern we would want users to mirror in their own projects. It is unusual that it would be appropriate for a callee to apply buffering like this to an arbitrary W: Write that it receives, because there is no way to inspect whether W is already buffered, and we just waste time shuffling data across multiple buffers if it is. Typically a W: Write should just be written to directly by a callee, and the caller is in charge of buffering if they want it.

If we do keep buffering inside say, then SmallVec is an odd choice for it. For large output, this ends up spilling to a heap allocation, when a better approach would be to flush the buffer by writing the contents to writer when it fills up, and skip the heap allocation.

The point of this issue isn't to optimize say, it is only to take best advantage of the opportunity to provide novices with a good example that will serve them well in their own future work, which the current approach in this crate doesn't do.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions