-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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 allocating the --help
message
#12243
Avoid allocating the --help
message
#12243
Conversation
48eea74
to
ebd1c5d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In my opinion, this isn't really needed.
Yes, it's true, that format
uses the heap, but performance wise it's neglectable.
I would prefer readability over "micro-performance-improvements" in this case.
ebd1c5d
to
8777d63
Compare
@TornaxO7 I have just reverted the changes in |
Eh, I'm not a maintainer, so you needn't see my review as "mandatory". Those are just my two cents :> |
Yeah generally there's a tradeoff in refactoring for a performance gain. We typically try to avoid refactors unless they bring some concrete benefits. It'd be nice to not allocate this non-trivially-small string - previously we allocated it even when not printing the help message - and this block barely every changes so I'll merge this. |
--help
message
format!
macro is very powerful, it introduces overhead of memory allocation on the heap compared to&str
. Therefore, whenformat!
macro is unnecessary, we should avoid using it.