Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

io::Error::new(_, string literal) allocates Box<Box<String>> and copies the string #83352

Open
m-ou-se opened this issue Mar 21, 2021 · 4 comments
Assignees
Labels
A-io Area: std::io, std::fs, std::net and std::path T-libs Relevant to the library team, which will review and decide on the PR/issue.

Comments

@m-ou-se
Copy link
Member

m-ou-se commented Mar 21, 2021

It's very common to use io::Error::new(kind, "some message"), including in many places in std. However, that will create a io::Error containing a Repr::Custom containing a Box<Custom> containing a Box<dyn Error + ..> containing a StringError containing a String containing a copy of the message. 🙃

Three allocations to just return an error message which is in some cases not even more helpful than just the ErrorKind, is not great. We should find a way to improve this.

@m-ou-se m-ou-se added T-libs Relevant to the library team, which will review and decide on the PR/issue. A-io Area: std::io, std::fs, std::net and std::path labels Mar 21, 2021
@m-ou-se m-ou-se self-assigned this Mar 21, 2021
@jyn514
Copy link
Member

jyn514 commented Mar 22, 2021

I think this is a duplicate of #82812 (or vice versa).

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Mar 23, 2021
Add internal io::Error::new_const to avoid allocations.

This makes it possible to have a io::Error containing a message with zero allocations, and uses that everywhere to avoid the *three* allocations involved in `io::Error::new(kind, "message")`.

The function signature isn't perfect, because it needs a reference to the `&str`. So for now, this is just a `pub(crate)` function. Later, we'll be able to use `fn new_const<MSG: &'static str>(kind: ErrorKind)` to make that a bit better. (Then we'll also be able to use some ZST trickery if that would result in more efficient code.)

See rust-lang#83352
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Mar 23, 2021
Add internal io::Error::new_const to avoid allocations.

This makes it possible to have a io::Error containing a message with zero allocations, and uses that everywhere to avoid the *three* allocations involved in `io::Error::new(kind, "message")`.

The function signature isn't perfect, because it needs a reference to the `&str`. So for now, this is just a `pub(crate)` function. Later, we'll be able to use `fn new_const<MSG: &'static str>(kind: ErrorKind)` to make that a bit better. (Then we'll also be able to use some ZST trickery if that would result in more efficient code.)

See rust-lang#83352
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Mar 24, 2021
Add internal io::Error::new_const to avoid allocations.

This makes it possible to have a io::Error containing a message with zero allocations, and uses that everywhere to avoid the *three* allocations involved in `io::Error::new(kind, "message")`.

The function signature isn't perfect, because it needs a reference to the `&str`. So for now, this is just a `pub(crate)` function. Later, we'll be able to use `fn new_const<MSG: &'static str>(kind: ErrorKind)` to make that a bit better. (Then we'll also be able to use some ZST trickery if that would result in more efficient code.)

See rust-lang#83352
@m-ou-se
Copy link
Member Author

m-ou-se commented Mar 27, 2021

The usages in std have been fixed by #83353

However, we still need to find a way to improve this for io::Errors created in other crates. We can't fix io::Error::new as explained here, so that'd have to be some new function or type, unfortunately. :(

@joshtriplett
Copy link
Member

joshtriplett commented Jul 12, 2023

Should we consider providing a stable io::Error::new_const, or something similar to it?

@m-ou-se
Copy link
Member Author

m-ou-se commented Jul 13, 2023

It'd have to be const fn new_const<const MSG: &str>() or const fn new_const(msg: &'static &'static str), so that's not very convenient.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-io Area: std::io, std::fs, std::net and std::path T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

3 participants