sharkdp / hexyl

A command-line hex viewer
Apache License 2.0
8.92k stars 227 forks source link

Change `ansi_term` to `anstyle` #176

Closed sharifhsn closed 1 year ago

sharifhsn commented 1 year ago

The ansi_term crate has been declared deprecated, so I've replaced it with the maintained crate anstyle which is recommended. This PR does three things:

  1. Replaces ansi_term with anstyle wherever necessary.

  2. Fixes #156 by using 4-bit ANSI colors everywhere. The gray color looks exactly the same as when using BrightBlack, so there should be no need to introduce multiple color modes.

  3. Increases performance significantly when printing with color.

As a quick review on how ANSI color codes work in the terminal, there are prefix codes that change the color/style, and a suffix code that resets the color/style to the original.

Previously, every single element would be colored with its own prefix and suffix, regardless of whether the next element had the same color or not. This meant that colored output wrote many more characters to the screen than was strictly necessary.

This PR writes the color prefix only when the color has changed, and writes the reset suffix only when a separator is reached (since separators are not colored).

In the best-case where most or all of the file is the same color e.g. a text file, this is almost 3x faster in real time. In the worst-case where the color changes often e.g. random data, this is almost 2x faster in real time.

hyperfine does not show this because it pipes all output to /dev/null, but this effect is apparent if you use the time command in the terminal. The hyperfine measurement is significantly slower, but as you have noted this is an artificial measurement.

Command Mean [ms] Min [ms] Max [ms] Relative
hexyl randomdata 209.8 ± 4.6 203.3 223.4 1.09 ± 0.06
target/release/hexyl randomdata 192.3 ± 10.4 184.0 227.9 1.00

In the real terminal the results are different, which you can test yourself using time.

sharkdp commented 1 year ago

Thank you very much!

In some of my other repositories, it was suggested to switch to nu-ansi-term. What are the differences?

sharkdp commented 1 year ago

hyperfine does not show this because it pipes all output to /dev/null

That's what hyperfines --output <WHERE> option is for:

        --output <WHERE>
            Control where the output of the benchmark is redirected. <WHERE> can be:

            null: Redirect output to /dev/null (the default). Note that some programs like
            'grep' detect when standard output is /dev/null and apply certain
            optimizations. To avoid that, consider using '--output=pipe'.

            pipe: Feed the output through a pipe before discarding it.

            inherit: Don't redirect the output at all (same as '--show-output').

            <FILE>: Write the output to the given file.

You can use --output=inherit to measure hexyls colorization in combination with terminal rendering speed. By the way, on the hyperfine issue tracker, we're even thinking about a --output=pty option (or similar) which would attach the output to some pseudo terminal. Exactly for purposes like this one.

sharifhsn commented 1 year ago

Thank you very much!

In some of my other repositories, it was suggested to switch to nu-ansi-term. What are the differences?

To be honest, I just picked the first option that was suggested on the advisory. From a cursory glance, nu-ansi-term seems to be a drop-in replacement with similar convenience methods, whereas anstyle is a lower-level API for ANSI color codes that maintains compatibility between different crates.

Currently, none of these libraries expose direct access to the color codes, instead returning an impl Display. In my first write, I rendered each code at the site, which created a 3x performance penalty. To avoid the allocation, this requires an awkward workaround where I have to render the codes as strings first globally and then refer to them later. I was planning on raising an issue to add these to the public API, since I'd rather not hardcode these values.

sharifhsn commented 1 year ago

That's what hyperfines --output <WHERE> option is for:

Oh, I didn't know about this! This is very useful and I should have been using it when benchmarking performance to begin with...

Command Mean [s] Min [s] Max [s] Relative
hexyl randomdata 2.396 ± 0.057 2.322 2.476 1.74 ± 0.06
target/release/hexyl randomdata 1.377 ± 0.033 1.327 1.415 1.00
Command Mean [s] Min [s] Max [s] Relative
hexyl ascii.txt 2.266 ± 0.042 2.180 2.332 2.74 ± 0.14
target/release/hexyl ascii.txt 0.828 ± 0.039 0.757 0.901 1.00

Here's proof for the claims I made about random and text data.

sharkdp commented 1 year ago

Cool - thank you for the benchmark results. Is this not ready for review yet?

sharifhsn commented 1 year ago

I wanted to test a different way of writing the Color prefixes that doesn't use once_cell.

sharifhsn commented 1 year ago

This PR is ready to merge. It will probably cause conflicts with #170 so I think it would be best to merge that first, then I can make changes to this one so it does not conflict.

sharkdp commented 1 year ago

Thank you, sounds good.

sharkdp commented 1 year ago

170 has been merged now. Thank you for your help @sharifhsn.

sharkdp commented 1 year ago

Thank you very much. This looks great. I know this will be next-to-impossible to properly test automatically (because the ANSI escape sequence output changed), but I trust you did some manual checks to make sure that the output still looks the same, color-wise?

sharifhsn commented 1 year ago

Yes, I checked on both Linux and Windows (I don't have access to macOS).