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

Avoid allocation in Display implementations #113

Open
luukvanderduim opened this issue Jul 10, 2023 · 1 comment · May be fixed by #200
Open

Avoid allocation in Display implementations #113

luukvanderduim opened this issue Jul 10, 2023 · 1 comment · May be fixed by #200

Comments

@luukvanderduim
Copy link
Collaborator

Sometimes we allocate when implementing Display where we do not need to, for example by using format! which will create a String which is heap allocated.
This can be undesirable for users and may negatively impact performance.

Display is the "Format trait for an empty format, {}.". Anywhere "{our_type}" is found, the our_type's Display::fmt() is called,
to create or add to a Formatter of the caller.

In almost all instances we could:

enum State {
    SomeState,
   SomeOtherState,
    ...
}

impl fmt::Display for State {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
           let sring_slice = match self {
           SomeState => "some state",
           SomeOtherState => "some other state",
           _ => "unknown state",
           };
           f.write_str(string_slice)
    }
}

As far as I understood, Formatter::write_str will not allocate unless the string slice is atypically large (I've read over 4Kb somewhere, 8Kb elsewhere).

If we do want to compose in our Display implementations, we can do without format! by using use format_args! on a stack allocated [u8] buffer instead and write! that buffer to the Formatter and avoid necessity of allocating during formatting.

@TTWNO
Copy link
Member

TTWNO commented Feb 29, 2024

I can think of one case where this would be an issue: AtspiError.

We use formatting to create the Display impl. Can you think of a way around this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants