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

Fix confusing smart console output from concurrent builds #1320

Closed
wants to merge 1 commit into from
Closed

Fix confusing smart console output from concurrent builds #1320

wants to merge 1 commit into from

Conversation

davezarzycki
Copy link

Developers tend to blame the last printed line when a build takes too
long. Unfortunately, when building concurrently, the last printed line
may have actually finished a long time ago. Under the current system,
ninja does not update the status line to reflect what jobs are still
running. This change makes ninja always print the longest running job
instead. In other words, the likely build bottlenecks.

Developers tend to blame the last printed line when a build takes too
long. Unfortunately, when building concurrently, the last printed line
may have actually finished a long time ago. Under the current system,
ninja does not update the status line to reflect what jobs are still
running. This change makes ninja always print the longest running job
instead. In other words, the likely build bottlenecks.
@davezarzycki
Copy link
Author

Hi @nico – Any chance you could please accept or reject this? Thanks!

@nico nico mentioned this pull request Apr 5, 2018
nico added a commit to nico/ninja that referenced this pull request Apr 5, 2018
Developers tend to blame the last printed line when a build takes too
long. Unfortunately, when building concurrently, the last printed line
may have actually finished a long time ago. Under the current system,
ninja does not update the status line to reflect what jobs are still
running. This change makes ninja always print the oldest still running job
instead. In other words, the likely build bottlenecks.

Patch from David Zarzycki, originally uploaded at ninja-build#1320.
@nico
Copy link
Collaborator

nico commented Apr 5, 2018

First, sorry about the long delay for getting around to this, I was away for a bit (cf https://groups.google.com/forum/#!topic/ninja-build/yUOLRh19M-w).

Thanks for the patch, this seems like a really clean and clever solution to this problem. I merged it in #1405 (with attribution).

@davezarzycki
Copy link
Author

Neat-o! Thanks!

projectgus pushed a commit to projectgus/ninja that referenced this pull request Apr 26, 2018
Developers tend to blame the last printed line when a build takes too
long. Unfortunately, when building concurrently, the last printed line
may have actually finished a long time ago. Under the current system,
ninja does not update the status line to reflect what jobs are still
running. This change makes ninja always print the oldest still running job
instead. In other words, the likely build bottlenecks.

Patch from David Zarzycki, originally uploaded at ninja-build#1320.
@jhasse
Copy link
Collaborator

jhasse commented Nov 5, 2018

Hi! Due to #1418, I needed to revert this, sorry :/
nico also noticed a performance regression, see #1405 (comment).

Maybe we can think of a solution which avoids those two issues. For #1418 you can use ./misc/output_test.py now to test it btw.

To keep the performance it might be a good idea to wait a second or two, before updating the status line due to a finished edge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants