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

Ninja does not update elapsed time in status for long build commands #2515

Open
digit-google opened this issue Oct 29, 2024 · 1 comment
Open
Labels
Milestone

Comments

@digit-google
Copy link
Contributor

NINJA_STATUS now supports new formatters, such as %e or %w, which print the elapsed time in seconds.

Their usefulness is however severely limited, because Ninja, still only updates its status when a command completes.
This results in poor user experience in smart terminals when long commands dominate a build (e.g. Rust link operations). Consider this example build plan:

rule sleep_then_print
  command = sleep 10 && echo done

build all: sleep_then_print

Running the command NINJA_STATUS="[%w] " ninja in a terminal results in the line:

[00:00] sleep 10 && echo done

Being printed for 10 seconds, without the time being updated. Then on completion, it is overwritten into:

[00:10] sleep 10 && echo done
done

The root of the problem is that SubprocessSet::DoWork() is used to wait for events during the build, and only returns when it detects that a process completed, or in the case of a user interruption.

To fix this issue the following changes are proposed:

  • Modify SubprocessSet::DoWork() to accept a timeout parameter in milliseconds, and return an enum to distinguish between timeout / process completion / user interruption.

  • Add a Status::Refresh(int64_t) method, and implement it in StatusPrinter::Refresh() to update the status when in interactive terminals.

  • Modify the RealCommandRunner function that calls DoWork() to handle timeouts and call status->Refresh() in this case.
    Current prototype at master...digit-google:ninja:periodic-status-updates

Note that doing this opens the door to other improvements that could appear later, such as:

  • Printing a table of running commands under the status line, to better understand what's going on.

  • Polling for new jobserver slots appearing in the pool to start new tasks as soon as possible, as explained in this comment, though ideally, it would be nicer if this could be handled as a new type of event for DoWork().

@jhasse
Copy link
Collaborator

jhasse commented Oct 31, 2024

Also see #111.

@jhasse jhasse added this to the 2.0.0 milestone Oct 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants