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.

Sign up for GitHub

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

Jump to bottom

Faster elide middle implementation #2487

Merged
merged 5 commits into from
Sep 2, 2024
Merged

Faster elide middle implementation #2487

merged 5 commits into from
Sep 2, 2024

Commits on Sep 2, 2024

  1. Rewrite ElideMiddle.ElideAnsiEscapeCodes test. 

    Use C string macros to hold ANSI escape sequences in order
to make the test much easier to understand and follow.

NOTE: This does not change the test in any way!
    @digit-google
    digit-google committed Sep 2, 2024
    Configuration menu
    Copy the full SHA
    60f901e View commit details
    Browse the repository at this point in the history

  2. Move ElideMiddle() to its own source file. 

    Move the function to its own source file since it is
no longer trivial (and will get optimized in future
patches).

+ Add a new ElideMiddleInPlace() function that doesn't
  do anything if no elision is required + use it in
  status.cc.

+ Add a new elide_middle_perftest program to benchmark
  the performance of the function.

  On my machine, the 'avg' result is 159.9ms.
    @digit-google
    digit-google committed Sep 2, 2024
    Configuration menu
    Copy the full SHA
    2d7ee52 View commit details
    Browse the repository at this point in the history

  3. Faster and more correct ElideMiddle with ANSI sequences 

    Add a fast-path for the case where there is no escape sequence
in the input, that avoids un-necessary string allocations.

Properly handle ANSI color sequences that appear in the elided
part of the input string. They *must* be preserved to ensure
that the right span is printed with the right color.

For example, consider the following input:

    |GREEN|aaaaaaaaaaaaa|RED|bbbbbbbbbbb|BLACK|

With different levels of elision:

    max_width=0   -> |GREEN|RED|BLACK| instead of ""

    max_width=5   -> |GREEN|a..|RED|b|BLACK|
      (this was |GREEN|a..b|BLACK| before this fix)

Moreoever, do not call std::regex_iterator::str() to avoid
un-necessary string allocations and concatenations.

With this patch, 'elide_middle_perftest' goes from 159.9ms
to 87.1 ms on my machine (x1.8 faster)
    @digit-google
    digit-google committed Sep 2, 2024
    Configuration menu
    Copy the full SHA
    ee9a296 View commit details
    Browse the repository at this point in the history

  4. ElideMiddle: Get rid of std::regex usage and heap allocations. 

    Get rid of std::regex and std::vector usage in ElideMiddleInPlace()
function. These are replaced with custom iterator classes that do
not perform any memory allocation at runtime.

Instead the input string will be parsed twice (once to determine the
visible width, and another time to create the elided result string).

With this patch, elide_middle_perftest goes from 87.1ms to 15.0ms
on my machine (x5.8 faster!). This also removes 65 kiB from the
stripped Linux binary for Ninja (!)
    @digit-google
    digit-google committed Sep 2, 2024
    Configuration menu
    Copy the full SHA
    5dd9b78 View commit details
    Browse the repository at this point in the history

  5. StatusPrinter: only strip ANSI sequences when needed. 

    Just check for the ESC character in the `output` string,
if none is present, or if the terminal supports color,
avoid unnecessary busy work (string allocations and
parsing).
    @digit-google
    digit-google committed Sep 2, 2024
    Configuration menu
    Copy the full SHA
    1ba5570 View commit details
    Browse the repository at this point in the history