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

Build edges that previously failed asap #60

Closed
wants to merge 2 commits into from

Conversation

justinsb
Copy link

This is a patch that implements a 'smarter' ordering when we have a choice of outputs to build, rather than just taking the first in the set. Here's the scenario: while developing, I hit a compilation error in a .cc file, and I have to change the .h file (and probably the .cc file as well). Because I changed the .h file, likely a large number of .cc files must be recompiled. However, I really would like to compile the .cc file that just broke first, so that I know asap whether I fixed the problem. This patch tries to do that.

The way it works is that it extends the build log to include the exit code (currently 0/1 based on the success flag), and when picking from the set of edges it picks the edge with the highest score (just simple iteration, no priority queue!) The score is currently very simple: it defaults to zero, but if a previous command on that edge failed, it is incremented.

This seems to be working pretty well for me; I feel that my fix / compile / refix cycles are faster :-)

@evmar
Copy link
Collaborator

evmar commented Jun 14, 2011

Interesting! I actually wrote a similar patch recently that attempted to instead prioritize the deepest node (in terms of the dependency tree) first, with the thought being it'd improve parallelism.

Here's an alternative approach for your idea that might simplify the implementation (no need to revise the log format): prioritize by input file last modification time. When you're in an edit/compile cycle you're probably editing the important files first, I think?

@justinsb
Copy link
Author

I think the timestamp approach is an improvement over 'random' selection; I might look at implementing this also (it's not as simple as it might first appear, because you probably want to fall back to the 2nd place timestamp when the 1st is a tie - consider the case when you edit first the .cc and then the .h file - for example changing a function to be const)

However, the exit code is also useful. Consider a complex .h file that uses templates, where a lot of .cc files import it, but only one of them uses the template in a problematic way and finds a bug I then fix in the .h file (only). Or even something as simple as a macro in a .h file where I need extra brackets. I want the 'problematic' .cc file to be compiled first; but timestamps don't capture the fact that the .cc file failed previously. So, even with timestamp detection, we'd still want this patch (or something like it).

@philipcraig
Copy link
Contributor

Hi Evan,

About "Interesting! I actually wrote a similar patch recently that attempted to instead prioritize the deepest node (in terms of the dependency tree) first, with the thought being it'd improve parallelism."

I think that would be a very interesting patch for my project. I mention it as we're seeing that we have thousands of .cpp files ready to build, but a "long tail" of links. The reason for that tail is that ninja has no concept of prioritising the "deepest node" build steps, so even links that should ideally happen early don't happen until that library's object files are completed, and those .cpp files are not given any special priority.

Will you submit that patch anytime soon?

@evmar
Copy link
Collaborator

evmar commented Jul 27, 2012

I think I lost my patch that prioritizes the deepest node. :(

Converted this into #376

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.

3 participants