-
Notifications
You must be signed in to change notification settings - Fork 45
Fix to no copy when format with yansi_term #30
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
Conversation
@botika what's your take on yansi-term
vs termcolor
? @brendanzab says that termcolor
may be a better long term bet.
I'm happy to land this PR, but I'm also curious on your opinion on that :)
The library is very simple, just enter a prefix and a suffix. So it does not need much maintenance nor can it innovate significantly. With the fork in the ansi_term
I have left the minimum to fulfill its function as quickly as possible
The reason for the PR is to remove all unnecessary mallocs
in the formatter.
I understand. I'm just curious on your take on long-term replacing ansi_term
or yansi_term
with termcolor
?
It is more optimal implement fmt::Display
than io::Write
. It forces you to use his buffer or stream output. I think the only thing this library should do is put a prefix and a suffix around the format flow, and let the user use whatever writer they want. So you couldn't use fmt::Formatter
for output stream. And I am unaware of compatibility outside of unix systems
I've been looking at termcolor
in more depth, just as I expected, it's forced to reallocate, here allocate to stack and here allocate to writer (wherever the writer writes).
So, the main problems of termcolor
are unnecessary use of memory and lack of abstraction of the writer
I updated the write bench, the results between io::Write
and fmt::Display
are:
Teams io::Write time: [423.37 ns 430.62 ns 436.27 ns]
Teams fmt::Write time: [291.71 ns 292.90 ns 294.35 ns]
The results are clear. The fastest way to write in Rust is to implement fmt::Display
.
Thank you!
This PR eliminates the need to reallocate while formatting.
For this I have forked the
ansi_term
library to refactor tofmt::Display
allowing to introduce flow with a closure that takes the current formatter as argument.It will depend on
yansi-term
instead ofansi_term
, which I will keep.